Re: [PATCH 3/3] trace: add GIT_TRACE_STDIN

[Jeff King <peff@peff.net>]

On Wed, Jun 17, 2015 at 05:04:04PM +0700, Duy Nguyen wrote:

> I wonder if we could do it a bit differently. Instead of
> GIT_TRACE_STDIN, I would add GIT_TRACE_HOOK that points to a script.
> Whenever a command is run via run-command interface, the actual
> command line to be executed would be "<hook script> <command>
> <arguments>".

Hmm, yeah, I like that. It's even more flexible, and it is much more
obvious why it works only for run-command. If we feed the resulting
"hooked" command to the shell, I think you could do:

  GIT_TRACE_HOOK='
    f() {
      case "$1 $2" in
      git pack-objects)
        tee /tmp/foo.out | "$@"
	;;
      esac
    }; f
  '

That is not 100% correct (you would miss "git --some-arg pack-objects"),
but it is probably fine in practice for debugging sessions. It is a bit
more complicated to use, but I really like the flexibility (I can
imagine that "GIT_TRACE_HOOK=gdbserver localhost:1234" would come in
handy).

> Because this script is given full command line, it can decide to trace
> something if the command name is matched (or arguments are matched) or
> just execute the original command. It's more flexible that trace.*
> config keys. We also have an opportunity to replace builtin commands,
> like pack-objects, in command pipeline in fetch or push with something
> else, to inject errors or whatever. It can be done manually, but it's
> not easy or convenient.

My other motive for trace.* was that we could have something like
"trace.prune", and have git-prune provide verbose debugging information.
We have custom patches like that on GitHub servers, which we've used to
debug occasional weirdness (e.g., you find that an object is missing
from a repo, but you have no clue why it went away; was it never there,
did somebody prune it, did it get dropped from a pack?).

I can send those upstream, but it would be nice not to introduce a
totally separate tracing facility when trace_* is so close. But it
needs:

  1. To be enabled by config, not environment.

  2. To support some basic output filename flexibility so the output can
     be organized (we write the equivalent of GIT_TRACE_FOO to
     $GIT_DIR/ghlog_foo/YYYY-MM-DD/YYYY-MM-DDTHH:MM:SS.PID).

For (1), we could just load trace.* in git_default_config; you couldn't
use it with any "early" tracing that happens before then, but I think in
practice it would be fine for most traces.

For (2), I think we could accomplish that with %-placeholders (like my
earlier patch), and the ability to write relative paths into $GIT_DIR
(again, you couldn't do this for "early" traces, but you could for other
stuff).

Or we could just do nothing. I'm not sure if anybody else is actually
interested in verbose-logging patches like these.

-Peff