threads / patch / 48228

patchMake running git under other debugger-like programs easy

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

## tl;dr

12 messages between Apr 5, 2018 and Apr 26, 2018. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Elijah Newren· Apr 5, 2018, 17:49 UTC · lore
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(-)
Show changes to wrap-for-bin.sh +1 −1
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
Johannes Schindelin· Apr 5, 2018, 19:57 UTC · re: Elijah Newren · lore

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

Hi Elijah,
On Thu, 5 Apr 2018, Elijah Newren wrote:
Show 9 quoted lines
> 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

Elijah Newren· Apr 5, 2018, 21:16 UTC · re: Johannes Schindelin · lore

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

Hi Johannes,

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

Show 52 quoted lines
> 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

Johannes Schindelin· Apr 6, 2018, 10:38 UTC · re: Elijah Newren · lore

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

Hi Elijah,
On Thu, 5 Apr 2018, Elijah Newren wrote:
Show 59 quoted lines
> 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?
Show 9 quoted lines
> > 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.
Show 6 quoted lines
> > (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

Elijah Newren· Apr 6, 2018, 22:36 UTC · re: Johannes Schindelin · lore

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

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>
Show 9 quoted lines
>> 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.
Show 21 quoted lines
>> 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'.

Show 10 quoted lines
>> > (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.
Elijah Newren· Apr 9, 2018, 18:51 UTC · re: Elijah Newren · lore

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

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(-)
Show changes to 2 files +33 −10

t/test-lib-functions.sh, wrap-for-bin.sh

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
Johannes Schindelin· Apr 9, 2018, 21:19 UTC · re: Elijah Newren · lore

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

Hi Elijah,
On Mon, 9 Apr 2018, Elijah Newren wrote:
Show 23 quoted lines
> 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`...

Show 40 quoted lines
> +		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

Elijah Newren· Apr 10, 2018, 00:48 UTC · re: Johannes Schindelin · lore

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

[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:

Show 13 quoted lines
> 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.
Show 9 quoted lines
>> +*)
>> +     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

Johannes Schindelin· Apr 10, 2018, 08:26 UTC · re: Elijah Newren · lore

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

Hi Elijah,
On Mon, 9 Apr 2018, Elijah Newren wrote:
Show 19 quoted lines
> 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.

Show 5 quoted lines
> 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.
Show 12 quoted lines
> >> +*)
> >> +     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.

Show 11 quoted lines
> 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

Elijah Newren· Apr 24, 2018, 23:46 UTC · re: Johannes Schindelin · lore

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

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(-)
Show changes to 2 files +33 −10

t/test-lib-functions.sh, wrap-for-bin.sh

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
Johannes Schindelin· Apr 25, 2018, 07:25 UTC · re: Elijah Newren · lore

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

Hi Elijah,
On Tue, 24 Apr 2018, Elijah Newren wrote:
Show 19 quoted lines
> 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

Junio C Hamano· Apr 26, 2018, 01:56 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 10 quoted lines
>> ...
>> 
>> 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.

← back to recent threads