Re: [PATCH 2/2] http.c: implements the GIT_CURL_DEBUG environment variable

[Elia Pinto <gitter.spiros@gmail.com>]

2016-04-01 17:03 GMT+02:00 Ramsay Jones <ramsay@ramsayjones.plus.com>:
>
>
> On 01/04/16 11:44, Elia Pinto wrote:
>> Implements the GIT_CURL_DEBUG environment variable to allow a greater
>> degree of detail of GIT_CURL_VERBOSE, in particular the complete
>> transport header and all the data payload exchanged.
>> It might be useful if a particular situation could require a more
>> thorough debugging analysis.
>>
>> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
>> ---
>>  http.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/http.c b/http.c
>> index dfc53c1..079779d 100644
>> --- a/http.c
>> +++ b/http.c
> [snip]
>
>> @@ -532,7 +623,11 @@ static CURL *get_curl_handle(void)
>>                       "your curl version is too old (>= 7.19.4)");
>>  #endif
>>
>> -     if (getenv("GIT_CURL_VERBOSE"))
>> +     if (getenv("GIT_CURL_DEBUG")) {
>> +             curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
>> +             curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
>> +             curl_easy_setopt(result, CURLOPT_DEBUGDATA, NULL);
>> +     } else if (getenv("GIT_CURL_VERBOSE"))
>>               curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
>>
>>       curl_easy_setopt(result, CURLOPT_USERAGENT,
>>
>
> Again, maybe something like:
>
> if (getenv("GIT_CURL_VERBOSE")) {
>         curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
>         if (getenv("GIT_CURL_DEBUG"))
>                 curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
> }
>
> Although that does make GIT_CURL_DEBUG subordinate to GIT_CURL_VERBOSE.
> So, that may not be desired ...

Thank you. But actually it is not a desirable change, for me almost, I
prefer that the two definitions are independent. And it is true the
opposite: if it is defined the curl DEBUG flag then it is implicitly
defined the curl VERBOSE flag, because it is a prerequisite of the
DEBUG functionality.

Thanks in any case for the review.

Best
>
> ATB,
> Ramsay Jones
>
>