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

[Elijah Newren <newren@gmail.com>]

[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