Re: [PATCH v6 1/1] http: add support selecting http version

2018-11-08 Thread Junio C Hamano
Eric Sunshine  writes:

>> @@ -284,6 +285,9 @@ static void process_curl_messages(void)
>>  static int http_options(const char *var, const char *value, void *cb)
>>  {
>> +   if (!strcmp("http.version",var)) {
>
> Style: space after comma
>
>> +   return git_config_string(_http_version, var, value);
>> +   }
>> @@ -806,6 +834,16 @@ static CURL *get_curl_handle(void)
>> +if (curl_http_version) {
>> +   long opt;
>> +   if (!get_curl_http_version_opt(curl_http_version, )) {
>> +   /* Set request use http version */
>> +   curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);
>
> Style: space after comma
>
>> +   }
>> +}

Thanks, both.  This is almost done, I think.


Re: [PATCH v6 1/1] http: add support selecting http version

2018-11-08 Thread Junio C Hamano
"Force Charlie via GitGitGadget"  writes:

> +#if LIBCURL_VERSION_NUM >=0x072f00
> +static int get_curl_http_version_opt(const char *version_string, long *opt)
> +{
> + int i;
> + static struct {
> + const char *name;
> + long opt_token;
> + } choice[] = {
> + { "HTTP/1.1", CURL_HTTP_VERSION_1_1 },
> + { "HTTP/2", CURL_HTTP_VERSION_2 }
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(choice); i++) {
> + if (!strcmp(version_string, choice[i].name)) {
> + *opt = choice[i].opt_token;
> + return 0;
> + }
> + }
> +

I wonder if we need to give a warning here about an unknown and
ignored value, by calling something like

warning("unknown value given to http.version: '%s'", version_string);

here.  We should not trigger noisy warning while reading the
configuration file looking for other variables unrelated to
http.version but this codepath is followed only when we know
we need to find out what value the variable is set to, so it
probably is a good thing to do.  

Thoughts?



Re: [PATCH v6 1/1] http: add support selecting http version

2018-11-08 Thread Eric Sunshine
On Thu, Nov 8, 2018 at 2:00 AM Force Charlie via GitGitGadget
 wrote:
> In order to give users the freedom to control the HTTP version,
> we need to add a setting to choose which HTTP version to use.
>
> Signed-off-by: Force Charlie 
> ---
> diff --git a/http.c b/http.c
> @@ -284,6 +285,9 @@ static void process_curl_messages(void)
>  static int http_options(const char *var, const char *value, void *cb)
>  {
> +   if (!strcmp("http.version",var)) {

Style: space after comma

> +   return git_config_string(_http_version, var, value);
> +   }
> @@ -806,6 +834,16 @@ static CURL *get_curl_handle(void)
> +if (curl_http_version) {
> +   long opt;
> +   if (!get_curl_http_version_opt(curl_http_version, )) {
> +   /* Set request use http version */
> +   curl_easy_setopt(result, CURLOPT_HTTP_VERSION,opt);

Style: space after comma

> +   }
> +}