Re: [PATCH 4/8] trace: refactor to support multiple env variables

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> Right now you turn all tracing off and on with GIT_TRACE. To
> support new types of tracing without forcing the user to see
> all of them, we will soon support turning each tracing area
> on with GIT_TRACE_*.
>
> This patch lays the groundwork by providing an interface
> which does not assume GIT_TRACE. However, we still maintain
> the trace_printf interface so that existing callers do not
> need to be refactored.

One thing I found missing in the patch description is a statement of the
overall design and expected usage.  It appears to me that the design is
built around the idea to give each named/namable area to be traceable its
own trace output file descriptor, so that different kinds of trace events
are sent to different files.

I however expect that majority of "trace only areas A and B" users would
want to see logs of events from these two areas in the same stream to see
them in the order they happened.  Perhaps you are envisioning that these
users would use redirection from the command line to send GIT_TRACE_A and
GIT_TRACE_B to the same place; that probably needs to be spelled out more
explicitly somewhere in the documentation, as that would be a more common
thing to do.

I think your [7/8] is kind of strange when viewed in that light.  Imagine
what would happen if you gave separate GIT_TRACE_* to each packet class,
instead of giving them a single umbrella variable GIT_TRACE_PACKET.  If
the user wants to see them all in a single stream, the same redirection
you would use to unify GIT_TRACE_A and GIT_TRACE_B can be used.

Instead, you have packet class prefix in the output so that later the
different kinds of packet events can be sifted out from the unified
output, even though they are forced to go to the same output stream.  In a
sense, you have two-tier classification system for traceable events (the
top layer that can be separated or merged at the file descriptor level,
and the bottom layer that can only be separated by looking at the prefix).

Is this necessarily a good thing (not a rhetorical question)?

To put it another and opposite way, I wonder if it would be better to
instead use a single output stream named by GIT_TRACE and add trace event
class prefix to the output for classes like SETUP and PACKET (again, not a
rhetorical question).

Also instead of wasting environment variable names, it might be a more
compact design from the user's point of view if we took a list of trace
event classes in a single environment variable, e.g.

	GIT_TRACE_CLASS=setup,packet \
        GIT_TRACE=/tmp/tr \
        git push

I dunno.