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

From: Jeff King <peff@peff.net>
Date: Jun 17, 2015, 19:10 UTC
Message-ID: <20150617191023.GA25304@peff.net>
In-Reply-To: <CACsJy8DaJOOu7PA8jL+Mu3HQua-TRmzN378Mek1HuWVmRzT+ww@mail.gmail.com>

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

Show 5 quoted lines

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

Show 7 quoted lines

> 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