Re: [PATCH 2/2] http, imap-send: stop using CURLOPT_VERBOSE

From: Jonathan Tan <jonathantanmy@google.com>
Date: May 12, 2020, 19:23 UTC
Message-ID: <20200512192300.203201-1-jonathantanmy@google.com>
In-Reply-To: <20200512191610.GB54565@coredump.intra.peff.net>

Show 17 quoted lines

> > +void http_trace_curl_no_data(void)
> > +{
> > +	trace_override_envvar(&trace_curl, "1");
> > +	trace_curl_data = 0;
> > +}
> 
> Would:
> 
>   setenv("GIT_TRACE_CURL", "1", 0);
>   setenv("GIT_TRACE_CURL_NO_DATA", "0", 0);
> 
> be simpler? Perhaps it makes the flow more convoluted as we'd go on to
> parse those variables, but it puts us on the same paths that we'd use if
> the user specified those things (and avoids the need for the special
> "override" function entirely).
> 
> Other than that nit, it seems very cleanly done.

Thanks for the review. I thought of doing that, but thought that it might add some latent complications - in particular, someone inspecting the environment variables of this running process might see some environment variables that they didn't set. But I'm OK either way.

Show 11 quoted lines

> PS I sometimes find the normal trace a bit verbose, but I do still want
>    to see data. Do others feel the same? Particularly I find the "SSL"
>    lines totally worthless (I guess maybe you could be debugging ssl
>    stuff, but that would be the exception, I'd think). Ditto the split
>    of data into two lines: one with the size and one with the actual
>    data.
> 
>    I dunno. I haven't been debugging any git-over-http stuff lately, so
>    it hasn't been bothering me. But I definitely have written perl
>    scripts to extract the data to a more readable format. Maybe it would
>    be easier if it had a few more knobs.

Data can be turned on using GIT_TRACE_CURL=1 and refraining from setting GIT_TRACE_CURL_NO_DATA. What knobs were you thinking of?