* [PATCH] Make running git under other debugger-like programs easy @ 2018-04-05 17:49 Elijah Newren 2018-04-05 19:57 ` Johannes Schindelin 2018-04-09 18:51 ` [PATCH v2] " Elijah Newren 0 siblings, 2 replies; 12+ messages in thread From: Elijah Newren @ 2018-04-05 17:49 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Make running git under other debugger-like programs easy 2018-04-05 17:49 [PATCH] Make running git under other debugger-like programs easy Elijah Newren @ 2018-04-05 19:57 ` Johannes Schindelin 2018-04-05 21:16 ` Elijah Newren 2018-04-09 18:51 ` [PATCH v2] " Elijah Newren 1 sibling, 1 reply; 12+ messages in thread From: Johannes Schindelin @ 2018-04-05 19:57 UTC (permalink / raw) To: Elijah Newren; +Cc: git 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Make running git under other debugger-like programs easy 2018-04-05 19:57 ` Johannes Schindelin @ 2018-04-05 21:16 ` Elijah Newren 2018-04-06 10:38 ` Johannes Schindelin 0 siblings, 1 reply; 12+ messages in thread From: Elijah Newren @ 2018-04-05 21:16 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Git Mailing List 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Make running git under other debugger-like programs easy 2018-04-05 21:16 ` Elijah Newren @ 2018-04-06 10:38 ` Johannes Schindelin 2018-04-06 22:36 ` Elijah Newren 0 siblings, 1 reply; 12+ messages in thread From: Johannes Schindelin @ 2018-04-06 10:38 UTC (permalink / raw) To: Elijah Newren; +Cc: Git Mailing List 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Make running git under other debugger-like programs easy 2018-04-06 10:38 ` Johannes Schindelin @ 2018-04-06 22:36 ` Elijah Newren 0 siblings, 0 replies; 12+ messages in thread From: Elijah Newren @ 2018-04-06 22:36 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Git Mailing List 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] Make running git under other debugger-like programs easy 2018-04-05 17:49 [PATCH] Make running git under other debugger-like programs easy Elijah Newren 2018-04-05 19:57 ` Johannes Schindelin @ 2018-04-09 18:51 ` Elijah Newren 2018-04-09 21:19 ` Johannes Schindelin 1 sibling, 1 reply; 12+ messages in thread From: Elijah Newren @ 2018-04-09 18:51 UTC (permalink / raw) To: git; +Cc: johannes.schindelin, 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Make running git under other debugger-like programs easy 2018-04-09 18:51 ` [PATCH v2] " Elijah Newren @ 2018-04-09 21:19 ` Johannes Schindelin 2018-04-10 0:48 ` Elijah Newren 0 siblings, 1 reply; 12+ messages in thread From: Johannes Schindelin @ 2018-04-09 21:19 UTC (permalink / raw) To: Elijah Newren; +Cc: git 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Make running git under other debugger-like programs easy 2018-04-09 21:19 ` Johannes Schindelin @ 2018-04-10 0:48 ` Elijah Newren 2018-04-10 8:26 ` Johannes Schindelin 0 siblings, 1 reply; 12+ messages in thread From: Elijah Newren @ 2018-04-10 0:48 UTC (permalink / raw) To: Johannes Schindelin, Git Mailing List [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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Make running git under other debugger-like programs easy 2018-04-10 0:48 ` Elijah Newren @ 2018-04-10 8:26 ` Johannes Schindelin 2018-04-24 23:46 ` [PATCH v3] " Elijah Newren 0 siblings, 1 reply; 12+ messages in thread From: Johannes Schindelin @ 2018-04-10 8:26 UTC (permalink / raw) To: Elijah Newren; +Cc: Git Mailing List 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] Make running git under other debugger-like programs easy 2018-04-10 8:26 ` Johannes Schindelin @ 2018-04-24 23:46 ` Elijah Newren 2018-04-25 7:25 ` Johannes Schindelin 0 siblings, 1 reply; 12+ messages in thread From: Elijah Newren @ 2018-04-24 23:46 UTC (permalink / raw) To: gitster; +Cc: git, Johannes.Schindelin, 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3] Make running git under other debugger-like programs easy 2018-04-24 23:46 ` [PATCH v3] " Elijah Newren @ 2018-04-25 7:25 ` Johannes Schindelin 2018-04-26 1:56 ` Junio C Hamano 0 siblings, 1 reply; 12+ messages in thread From: Johannes Schindelin @ 2018-04-25 7:25 UTC (permalink / raw) To: Elijah Newren; +Cc: gitster, git 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] Make running git under other debugger-like programs easy 2018-04-25 7:25 ` Johannes Schindelin @ 2018-04-26 1:56 ` Junio C Hamano 0 siblings, 0 replies; 12+ messages in thread From: Junio C Hamano @ 2018-04-26 1:56 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Elijah Newren, git 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. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-04-26 1:56 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-05 17:49 [PATCH] Make running git under other debugger-like programs easy Elijah Newren 2018-04-05 19:57 ` Johannes Schindelin 2018-04-05 21:16 ` Elijah Newren 2018-04-06 10:38 ` Johannes Schindelin 2018-04-06 22:36 ` Elijah Newren 2018-04-09 18:51 ` [PATCH v2] " Elijah Newren 2018-04-09 21:19 ` Johannes Schindelin 2018-04-10 0:48 ` Elijah Newren 2018-04-10 8:26 ` Johannes Schindelin 2018-04-24 23:46 ` [PATCH v3] " Elijah Newren 2018-04-25 7:25 ` Johannes Schindelin 2018-04-26 1:56 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).