[PATCH] Make running git under other debugger-like programs easy

[PATCH] Make running git under other debugger-like programs easy

[Elijah Newren]

This allows us to run git, when using the script from bin-wrappers, under
other programs.  A few examples:
   GIT_WRAPPER=nemiver git $ARGS
   GIT_WRAPPER="valgrind --tool=memcheck --track-origins=yes" git $ARGS

Yes, we already have GIT_TEST_GDB (which could potentially be replaced
with GIT_WRAPPER="gdb --args"), and a bunch of options for running
a testcase or multiple testcases under valgrind, but I find the extra
flexibility useful.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 wrap-for-bin.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
index 5842408817..1b34d44193 100644
--- a/wrap-for-bin.sh
+++ b/wrap-for-bin.sh
@@ -25,5 +25,5 @@ then
 	unset GIT_TEST_GDB
 	exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
 else
-	exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+	exec ${GIT_WRAPPER} "${GIT_EXEC_PATH}/@@PROG@@" "$@"
 fi
-- 
2.17.0.7.g0b50f94d69

Re: [PATCH] Make running git under other debugger-like programs easy

[Johannes Schindelin]

Hi Elijah,

On Thu, 5 Apr 2018, Elijah Newren wrote:

> This allows us to run git, when using the script from bin-wrappers, under
> other programs.  A few examples:
>    GIT_WRAPPER=nemiver git $ARGS
>    GIT_WRAPPER="valgrind --tool=memcheck --track-origins=yes" git $ARGS
> 
> Yes, we already have GIT_TEST_GDB (which could potentially be replaced
> with GIT_WRAPPER="gdb --args"), and a bunch of options for running
> a testcase or multiple testcases under valgrind, but I find the extra
> flexibility useful.

It would be even more useful if it could be made to work interactively,
too, by removing those redirections. The `debug` function does this
thusly:

debug () {
         GIT_TEST_GDB=1 "$@" <&6 >&5 2>&7
}

I wonder whether a better approach would be to add an optional argument to
that `debug` function (which is intended to have `git` as first argument,
anyway, or at least a command/function that does not start with a dash):

debug_aux () {
	shift
	"$@" <&6 >&5 2>&7
}

debug () {
	case "$1" in
	-d)
		shift &&
		GIT_TEST_GDB="$1" debug_aux "$@"
		;;
	--debugger=*)
		GIT_TEST_GDB="${1#*=}" debug_aux "$@"
		;;
	*)
		GIT_TEST_GDB=1 "$@" <&6 >&5 2>&7
		;;
	esac
}

... and then in wrap-for-bin.sh, we would replace the last lines

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

by

case "$GIT_TEST_GDB" in
'')
	exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
	;;
1)
	unset GIT_TEST_GDB
	exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
	;;
*)
	GIT_TEST_GDB_$$="$GIT_TEST_GDB"
	unset GIT_TEST_GDB
	exec $GIT_TEST_GDB_$$ "${GIT_EXEC_PATH}/@@PROG@@" "$@"
	;;
esac

or some such.

Then your magic "GIT_WRAPPER" invocation would become a bit more explicit:

    debug --debugger=nemiver git $ARGS
    debug -d "valgrind --tool=memcheck --track-origins=yes" git $ARGS

(In any case, "GIT_WRAPPER" is probably a name in want of being renamed.)

Ciao,
Dscho

Re: [PATCH] Make running git under other debugger-like programs easy

[Elijah Newren]

Hi Johannes,

On Thu, Apr 5, 2018 at 12:57 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:

> I wonder whether a better approach would be to add an optional argument to
> that `debug` function (which is intended to have `git` as first argument,
> anyway, or at least a command/function that does not start with a dash):
>
> debug_aux () {
>         shift
>         "$@" <&6 >&5 2>&7
> }
>
> debug () {
>         case "$1" in
>         -d)
>                 shift &&
>                 GIT_TEST_GDB="$1" debug_aux "$@"
>                 ;;
>         --debugger=*)
>                 GIT_TEST_GDB="${1#*=}" debug_aux "$@"
>                 ;;
>         *)
>                 GIT_TEST_GDB=1 "$@" <&6 >&5 2>&7
>                 ;;
>         esac
> }
>
> ... and then in wrap-for-bin.sh, we would replace the last lines
>
> if test -n "$GIT_TEST_GDB"
> then
>         unset GIT_TEST_GDB
>         exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> else
>         exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> fi
>
> by
>
> case "$GIT_TEST_GDB" in
> '')
>         exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
>         ;;
> 1)
>         unset GIT_TEST_GDB
>         exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
>         ;;
> *)
>         GIT_TEST_GDB_$$="$GIT_TEST_GDB"
>         unset GIT_TEST_GDB
>         exec $GIT_TEST_GDB_$$ "${GIT_EXEC_PATH}/@@PROG@@" "$@"
>         ;;
> esac
>
> or some such.

That all looks great to me.  But at this point, it seems like it's a
full rewrite and your patch to submit (which I'd be happy to endorse
in lieu of my own)...or do you want me to submit with you as author
and me as committer?

Also, a side question: if we go this route, do we want to rename
GIT_TEST_GDB to reflect its expanded usage?

> Then your magic "GIT_WRAPPER" invocation would become a bit more explicit:
>
>     debug --debugger=nemiver git $ARGS
>     debug -d "valgrind --tool=memcheck --track-origins=yes" git $ARGS

No, for most (60-80%?) of my invocations, I wouldn't be able to use
the debug function; only a minority of my uses are from within the
testsuite.  The rest are from the command line (I have
git/bin-wrappers/ in my $PATH), so the above suggestions would mean
that my invocation would become:

GIT_TEST_GDB="nemiver" git $ARGS
GIT_TEST_GDB="valgrind --tool-memcheck --track-origins=yes" git $ARGS

> (In any case, "GIT_WRAPPER" is probably a name in want of being renamed.)

Well, with your suggestion, it'd just be whatever that environment
variable is named.  I'm perfectly happy with something other than
GIT_WRAPPER (or GIT_TEST_GDB).  I'm not so good at coming up with such
myself, but maybe something like GIT_DEBUGGER or GIT_DEBUG_WITH?

Thanks,
Elijah

Re: [PATCH] Make running git under other debugger-like programs easy

[Johannes Schindelin]

Hi Elijah,

On Thu, 5 Apr 2018, Elijah Newren wrote:

> On Thu, Apr 5, 2018 at 12:57 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> 
> > I wonder whether a better approach would be to add an optional argument to
> > that `debug` function (which is intended to have `git` as first argument,
> > anyway, or at least a command/function that does not start with a dash):
> >
> > debug_aux () {
> >         shift
> >         "$@" <&6 >&5 2>&7
> > }
> >
> > debug () {
> >         case "$1" in
> >         -d)
> >                 shift &&
> >                 GIT_TEST_GDB="$1" debug_aux "$@"
> >                 ;;
> >         --debugger=*)
> >                 GIT_TEST_GDB="${1#*=}" debug_aux "$@"
> >                 ;;
> >         *)
> >                 GIT_TEST_GDB=1 "$@" <&6 >&5 2>&7
> >                 ;;
> >         esac
> > }
> >
> > ... and then in wrap-for-bin.sh, we would replace the last lines
> >
> > if test -n "$GIT_TEST_GDB"
> > then
> >         unset GIT_TEST_GDB
> >         exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> > else
> >         exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> > fi
> >
> > by
> >
> > case "$GIT_TEST_GDB" in
> > '')
> >         exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> >         ;;
> > 1)
> >         unset GIT_TEST_GDB
> >         exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> >         ;;
> > *)
> >         GIT_TEST_GDB_$$="$GIT_TEST_GDB"
> >         unset GIT_TEST_GDB
> >         exec $GIT_TEST_GDB_$$ "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> >         ;;
> > esac
> >
> > or some such.
> 
> That all looks great to me.  But at this point, it seems like it's a
> full rewrite and your patch to submit (which I'd be happy to endorse
> in lieu of my own)...

:-)

> or do you want me to submit with you as author and me as committer?

That would be my preference. I have not even tested what I wrote above...

> Also, a side question: if we go this route, do we want to rename
> GIT_TEST_GDB to reflect its expanded usage?

Sure. Probably GIT_TEST_DEBUGGER? Or GIT_TEST_DBG? Or GIT_TEST_DEBUG?

> > Then your magic "GIT_WRAPPER" invocation would become a bit more explicit:
> >
> >     debug --debugger=nemiver git $ARGS
> >     debug -d "valgrind --tool=memcheck --track-origins=yes" git $ARGS
> 
> No, for most (60-80%?) of my invocations, I wouldn't be able to use
> the debug function; only a minority of my uses are from within the
> testsuite.  The rest are from the command line (I have
> git/bin-wrappers/ in my $PATH),

Oy vey. bin-wrappers in your PATH? That's even worse than what I did in
the first two years of developing Git: I always ran `git` in-place.
However, I was bitten by a couple of bugs introduced while developing that
made it hard to recover (if I don't have a functional Git, I cannot use it
to go back to a working version, can I?). How do *you* deal with these
things?

> so the above suggestions would mean that my invocation would become:
> 
> GIT_TEST_GDB="nemiver" git $ARGS
> GIT_TEST_GDB="valgrind --tool-memcheck --track-origins=yes" git $ARGS

Right.

> > (In any case, "GIT_WRAPPER" is probably a name in want of being renamed.)
> 
> Well, with your suggestion, it'd just be whatever that environment
> variable is named.  I'm perfectly happy with something other than
> GIT_WRAPPER (or GIT_TEST_GDB).  I'm not so good at coming up with such
> myself, but maybe something like GIT_DEBUGGER or GIT_DEBUG_WITH?

I like both. Pick whatever you like, as long as it starts with `GIT_` and
is descriptive enough. Even `GIT_LAUNCH_THROUGH` would work, but
`GIT_DEBUGGER` seems to be the shortest that still makes sense.

Ciao,
Dscho

Re: [PATCH] Make running git under other debugger-like programs easy

[Elijah Newren]

Hi Dscho,

On Fri, Apr 6, 2018 at 3:38 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Thu, 5 Apr 2018, Elijah Newren wrote:
>> On Thu, Apr 5, 2018 at 12:57 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:

<snip>

>> That all looks great to me.  But at this point, it seems like it's a
>> full rewrite and your patch to submit (which I'd be happy to endorse
>> in lieu of my own)...
>
> :-)
>
>> or do you want me to submit with you as author and me as committer?
>
> That would be my preference. I have not even tested what I wrote above...

Sure, will do.

>> Also, a side question: if we go this route, do we want to rename
>> GIT_TEST_GDB to reflect its expanded usage?
>
> Sure. Probably GIT_TEST_DEBUGGER? Or GIT_TEST_DBG? Or GIT_TEST_DEBUG?
>
>> > Then your magic "GIT_WRAPPER" invocation would become a bit more explicit:
>> >
>> >     debug --debugger=nemiver git $ARGS
>> >     debug -d "valgrind --tool=memcheck --track-origins=yes" git $ARGS
>>
>> No, for most (60-80%?) of my invocations, I wouldn't be able to use
>> the debug function; only a minority of my uses are from within the
>> testsuite.  The rest are from the command line (I have
>> git/bin-wrappers/ in my $PATH),
>
> Oy vey. bin-wrappers in your PATH? That's even worse than what I did in
> the first two years of developing Git: I always ran `git` in-place.
> However, I was bitten by a couple of bugs introduced while developing that
> made it hard to recover (if I don't have a functional Git, I cannot use it
> to go back to a working version, can I?). How do *you* deal with these
> things?

I also have an older system git in /usr/bin; if things go sideways, I
just explicitly use '/usr/bin/git' instead of 'git'.

>> > (In any case, "GIT_WRAPPER" is probably a name in want of being renamed.)
>>
>> Well, with your suggestion, it'd just be whatever that environment
>> variable is named.  I'm perfectly happy with something other than
>> GIT_WRAPPER (or GIT_TEST_GDB).  I'm not so good at coming up with such
>> myself, but maybe something like GIT_DEBUGGER or GIT_DEBUG_WITH?
>
> I like both. Pick whatever you like, as long as it starts with `GIT_` and
> is descriptive enough. Even `GIT_LAUNCH_THROUGH` would work, but
> `GIT_DEBUGGER` seems to be the shortest that still makes sense.

Cool, GIT_DEBUGGER sounds good to me, I'll just proceed with it.

[PATCH v2] Make running git under other debugger-like programs easy

[Elijah Newren]

This allows us to run git, when using the script from bin-wrappers, under
other programs.  A few examples for usage within testsuite scripts:

   debug git checkout master
   debug --debugger=nemiver git $ARGS
   debug -d "valgrind --tool-memcheck --track-origins=yes" git $ARGS

Or, if someone has bin-wrappers/ in their $PATH and is executing git
outside the testsuite:

   GIT_DEBUGGER="gdb --args" git $ARGS
   GIT_DEBUGGER=nemiver git $ARGS
   GIT_DEBUGGER="valgrind --tool=memcheck --track-origins=yes" git $ARGS

There is also a handy shortcut of GIT_DEBUGGER=1 meaning the same as
GIT_DEBUGGER="gdb --args"

Original-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/test-lib-functions.sh | 24 ++++++++++++++++++++----
 wrap-for-bin.sh         | 19 +++++++++++++------
 2 files changed, 33 insertions(+), 10 deletions(-)

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" &&
+		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@@" "$@"
+	;;
+esac
-- 
2.15.0.1.gcd9d12fc4a.dirty

Re: [PATCH v2] Make running git under other debugger-like programs easy

[Johannes Schindelin]

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

Re: [PATCH v2] Make running git under other debugger-like programs easy

[Elijah Newren]

[Re-sending, making sure the annoying rich text mode isn't turned on
in gmail...]

Hi Dscho,

On Mon, Apr 9, 2018 at 2:19 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi Elijah,
>
> On Mon, 9 Apr 2018, Elijah Newren wrote:
>
>>  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`...

I guess we could instead do a
   export GIT_DEBUGGER="$2"
here.  Short of explicitly using 'export', I think we'd need another
environment variable.

I would have stuck with your original suggestion, except that by running:
    GIT_DEBUGGER="$1" debug_aux "$@"
GIT_DEBUGGER would be set within the debug_aux function and would NOT
be set once bin-wrappers/git was called, making git not run under the
debugger as desired.

>> +*)
>> +     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...)

You had a separate, per-process variable:
GIT_DEBUGGER_$$="$GIT_DEBUGGER"

The problem with that line is that the $$ apparently makes bash treat
it as a command to run rather than as a variable and a value with the
desire to set one to the other.  Prepending the line with 'declare' or
'local' or perhaps 'readonly' would fix that, but those aren't posix
and my reading suggested that while some other shells did support some
of those builtins, the support was somewhat spotty.  Since
wrap-for-bin.sh runs under /bin/sh, not /bin/bash, I didn't have a way
of making the per-process piece work and just fell back to a separate
variable.  Maybe we want to change that script to /bin/bash?

Do you see any alternatives I'm missing?

Thanks,
Elijah

Re: [PATCH v2] Make running git under other debugger-like programs easy

[Johannes Schindelin]

Hi Elijah,

On Mon, 9 Apr 2018, Elijah Newren wrote:

> On Mon, Apr 9, 2018 at 2:19 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Mon, 9 Apr 2018, Elijah Newren wrote:
> >
> >>  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`...
> 
> I guess we could instead do a
>    export GIT_DEBUGGER="$2"
> here.  Short of explicitly using 'export', I think we'd need another
> environment variable.

I suppose so. The GIT_DEBUGGER variable has to be exported *anyway*,
otherwise the bin-wrappers/* won't see it.

> I would have stuck with your original suggestion, except that by running:
>     GIT_DEBUGGER="$1" debug_aux "$@"
> GIT_DEBUGGER would be set within the debug_aux function and would NOT
> be set once bin-wrappers/git was called, making git not run under the
> debugger as desired.

Oh, so in debug_aux, you'd have to do some funny

	GIT_DEBUGGER="$GIT_DEBUGGER" "$@"

to export that variable to the process... Ugly.

> >> +*)
> >> +     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...)
> 
> You had a separate, per-process variable:
> GIT_DEBUGGER_$$="$GIT_DEBUGGER"

Right. And I guess it does not work because the _$$ is not expanded, you'd
have to eval it, and then you'd have to quote the right side
appropriately, and it's all not worth it.

> The problem with that line is that the $$ apparently makes bash treat
> it as a command to run rather than as a variable and a value with the
> desire to set one to the other.  Prepending the line with 'declare' or
> 'local' or perhaps 'readonly' would fix that, but those aren't posix
> and my reading suggested that while some other shells did support some
> of those builtins, the support was somewhat spotty.  Since
> wrap-for-bin.sh runs under /bin/sh, not /bin/bash, I didn't have a way
> of making the per-process piece work and just fell back to a separate
> variable.  Maybe we want to change that script to /bin/bash?
> 
> Do you see any alternatives I'm missing?

No, not really.

Well, let's stick with your patch (with s/DBG_FLAGS/GIT_DEBUGGER/).

Thanks,
Dscho

[PATCH v3] Make running git under other debugger-like programs easy

[Elijah Newren]

This allows us to run git, when using the script from bin-wrappers, under
other programs.  A few examples for usage within testsuite scripts:

   debug git checkout master
   debug --debugger=nemiver git $ARGS
   debug -d "valgrind --tool-memcheck --track-origins=yes" git $ARGS

Or, if someone has bin-wrappers/ in their $PATH and is executing git
outside the testsuite:

   GIT_DEBUGGER="gdb --args" git $ARGS
   GIT_DEBUGGER=nemiver git $ARGS
   GIT_DEBUGGER="valgrind --tool=memcheck --track-origins=yes" git $ARGS

There is also a handy shortcut of GIT_DEBUGGER=1 meaning the same as
GIT_DEBUGGER="gdb --args"

Original-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Elijah Newren <newren@gmail.com>
---

Finally getting back to this now that I have tied up all loose ends
with the directory rename detection series (or at least I hope they
are all tied up).

There is only one change since v2:
  s/DBG_FLAGS/GIT_DEBUGGER/, as suggested by Dscho


 t/test-lib-functions.sh | 24 ++++++++++++++++++++----
 wrap-for-bin.sh         | 19 +++++++++++++------
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index b895366fee..a407b09b48 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)
+		GIT_DEBUGGER="$2" &&
+		shift 2
+		;;
+	--debugger=*)
+		GIT_DEBUGGER="${1#*=}" &&
+		shift 1
+		;;
+	*)
+		GIT_DEBUGGER=1
+		;;
+	esac &&
+	GIT_DEBUGGER="${GIT_DEBUGGER}" "$@" <&6 >&5 2>&7
 }
 
 # Call test_commit with the arguments
diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
index 5842408817..95851b85b6 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@@" "$@"
+	;;
+esac
-- 
2.17.0.2.g31fed8301b

Re: [PATCH v3] Make running git under other debugger-like programs easy

[Johannes Schindelin]

Hi Elijah,

On Tue, 24 Apr 2018, Elijah Newren wrote:

> This allows us to run git, when using the script from bin-wrappers, under
> other programs.  A few examples for usage within testsuite scripts:
> 
>    debug git checkout master
>    debug --debugger=nemiver git $ARGS
>    debug -d "valgrind --tool-memcheck --track-origins=yes" git $ARGS
> 
> Or, if someone has bin-wrappers/ in their $PATH and is executing git
> outside the testsuite:
> 
>    GIT_DEBUGGER="gdb --args" git $ARGS
>    GIT_DEBUGGER=nemiver git $ARGS
>    GIT_DEBUGGER="valgrind --tool=memcheck --track-origins=yes" git $ARGS
> 
> There is also a handy shortcut of GIT_DEBUGGER=1 meaning the same as
> GIT_DEBUGGER="gdb --args"
> 
> Original-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Elijah Newren <newren@gmail.com>

Looks good to me!
Dscho

Re: [PATCH v3] Make running git under other debugger-like programs easy

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> ...
>> 
>> There is also a handy shortcut of GIT_DEBUGGER=1 meaning the same as
>> GIT_DEBUGGER="gdb --args"
>> 
>> Original-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> Signed-off-by: Elijah Newren <newren@gmail.com>
>
> Looks good to me!
> Dscho

Thanks, both.