Re: [PATCH 3/4] http: drop support for curl < 7.19.4

[Jeff King]

On Thu, Aug 10, 2017 at 02:36:41PM +0200, Mischa POSLAWSKY wrote:

> Jeff King wrote:
> > -#if LIBCURL_VERSION_NUM >= 0x071301
> > curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
> > -#elif LIBCURL_VERSION_NUM >= 0x071101
> > curl_easy_setopt(result, CURLOPT_POST301, 1);
> > -#endif
> 
> This seems to be an unintended behavioural change: the second condition
> wouldn't have applied previously and overrides the first option
> (equivalent to CURLOPT_POSTREDIR = CURL_REDIR_POST_301).

Thanks, you're right. I'll fix it in my re-roll.

-Peff

Re: [PATCH 3/4] http: drop support for curl < 7.19.4

[Mischa POSLAWSKY]

Jeff King wrote:
> -#if LIBCURL_VERSION_NUM >= 0x071301
>   curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
> -#elif LIBCURL_VERSION_NUM >= 0x071101
>   curl_easy_setopt(result, CURLOPT_POST301, 1);
> -#endif

This seems to be an unintended behavioural change: the second condition
wouldn't have applied previously and overrides the first option
(equivalent to CURLOPT_POSTREDIR = CURL_REDIR_POST_301).

-- 
Mischa

Re: [PATCH 3/4] http: drop support for curl < 7.19.4

[Jeff King]

On Wed, Aug 09, 2017 at 10:34:09AM -0700, Stefan Beller wrote:

> On Wed, Aug 9, 2017 at 5:02 AM, Jeff King  wrote:
> > Since v2.12.0, Git does not compile with versions of curl
> > older than 7.19.4. That version of curl is about 8 years
> > old. This means it may still be used in some distributions
> > with long-running support periods. But the fact that we
> > haven't received a single bug report about the compile-time
> > breakage implies that nobody cares about building recent
> > releases on such platforms.
> 
> I would not state it as bland, see how
> https://public-inbox.org/git/20170806233850.14711-1-ava...@gmail.com/
> came here to the mailing list.

Heh, I almost added "Or they are happy patching Git themselves". This
_does_ make patching Git harder for them, because now there are a lot
more spots to patch.

> > As discussed in the previous two commits, this cleans up the
> > code and gives a more realistic signal to users about which
> > versions of Git are actually tested (in particular, this
> > moves us past the potential use-after-free issues with curl
> > older than 7.17.0).
> 
> This is a good reason for this patch, though, so maybe just elide
> the "nobody cares" part?

I think I'd rather elaborate than elide. One of the reasons to split
this into multiple patches is that it's a ready-made patch for a
distributor to apply (in reverse) if they really want to.

-Peff

Re: [PATCH 3/4] http: drop support for curl < 7.19.4

[Stefan Beller]

On Wed, Aug 9, 2017 at 5:02 AM, Jeff King  wrote:
> Since v2.12.0, Git does not compile with versions of curl
> older than 7.19.4. That version of curl is about 8 years
> old. This means it may still be used in some distributions
> with long-running support periods. But the fact that we
> haven't received a single bug report about the compile-time
> breakage implies that nobody cares about building recent
> releases on such platforms.

I would not state it as bland, see how
https://public-inbox.org/git/20170806233850.14711-1-ava...@gmail.com/
came here to the mailing list.


> As discussed in the previous two commits, this cleans up the
> code and gives a more realistic signal to users about which
> versions of Git are actually tested (in particular, this
> moves us past the potential use-after-free issues with curl
> older than 7.17.0).

This is a good reason for this patch, though, so maybe just elide
the "nobody cares" part?

Thanks for these cleanups!

Re: [PATCH 3/4] http: drop support for curl < 7.19.4

[Jeff King]

On Wed, Aug 09, 2017 at 03:14:22PM +0200, Ævar Arnfjörð Bjarmason wrote:

> This whole series looks good to me. As I commented on in the thread you
> referenced in 0/4 I think this is the right trade-off, and people like
> me who occasionally compile git on older systems can just easily package
> a newer curl as well if we need it.
> 
> My reading of the curl history/docs is that you should squash this into
> this last patch. It's code that's now dead since we require
> 7.19.4.
> 
> CURLAUTH_DIGEST_IE was added in 7.19.3, and as a comment this squash
> removes indicates CURLOPT_USE_SSL hasn't been needed since 7.16.4:
> https://curl.haxx.se/libcurl/c/CURLOPT_USE_SSL.html

Thanks. Do you mind formatting this as a patch on top instead of a
squash? I think it's sufficiently subtle that it should be separate from
the main cleanup, which is just dropping our own internal #ifdefs.

I guess that would make reverting harder, though.

-Peff

Re: [PATCH 3/4] http: drop support for curl < 7.19.4

[Ævar Arnfjörð Bjarmason]

On Wed, Aug 09 2017, Jeff King jotted:

> Since v2.12.0, Git does not compile with versions of curl
> older than 7.19.4. That version of curl is about 8 years
> old. This means it may still be used in some distributions
> with long-running support periods. But the fact that we
> haven't received a single bug report about the compile-time
> breakage implies that nobody cares about building recent
> releases on such platforms.

This whole series looks good to me. As I commented on in the thread you
referenced in 0/4 I think this is the right trade-off, and people like
me who occasionally compile git on older systems can just easily package
a newer curl as well if we need it.

My reading of the curl history/docs is that you should squash this into
this last patch. It's code that's now dead since we require
7.19.4.

CURLAUTH_DIGEST_IE was added in 7.19.3, and as a comment this squash
removes indicates CURLOPT_USE_SSL hasn't been needed since 7.16.4:
https://curl.haxx.se/libcurl/c/CURLOPT_USE_SSL.html

diff --git a/http.c b/http.c
index 5280511c74..527bc56dc2 100644
--- a/http.c
+++ b/http.c
@@ -103,9 +103,7 @@ static int http_auth_methods_restricted;
 /* Modes for which empty_auth cannot actually help us. */
 static unsigned long empty_auth_useless =
CURLAUTH_BASIC
-#ifdef CURLAUTH_DIGEST_IE
| CURLAUTH_DIGEST_IE
-#endif
| CURLAUTH_DIGEST;

 static struct curl_slist *pragma_header;
@@ -706,10 +704,8 @@ static CURL *get_curl_handle(void)
if (curl_ftp_no_epsv)
curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);

-#ifdef CURLOPT_USE_SSL
if (curl_ssl_try)
curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
-#endif

/*
 * CURL also examines these variables as a fallback; but we need to 
query
diff --git a/http.h b/http.h
index 29acfe8c55..66d2d3c539 100644
--- a/http.h
+++ b/http.h
@@ -16,15 +16,6 @@

 #define DEFAULT_MAX_REQUESTS 5

-/*
- * CURLOPT_USE_SSL was known as CURLOPT_FTP_SSL up to 7.16.4,
- * and the constants were known as CURLFTPSSL_*
-*/
-#if !defined(CURLOPT_USE_SSL) && defined(CURLOPT_FTP_SSL)
-#define CURLOPT_USE_SSL CURLOPT_FTP_SSL
-#define CURLUSESSL_TRY CURLFTPSSL_TRY
-#endif
-
 struct slot_results {
CURLcode curl_result;
long http_code;

[PATCH 3/4] http: drop support for curl < 7.19.4

[Jeff King]

Since v2.12.0, Git does not compile with versions of curl
older than 7.19.4. That version of curl is about 8 years
old. This means it may still be used in some distributions
with long-running support periods. But the fact that we
haven't received a single bug report about the compile-time
breakage implies that nobody cares about building recent
releases on such platforms.

As discussed in the previous two commits, this cleans up the
code and gives a more realistic signal to users about which
versions of Git are actually tested (in particular, this
moves us past the potential use-after-free issues with curl
older than 7.17.0).

Signed-off-by: Jeff King 
---
 http.c | 46 --
 http.h |  4 
 2 files changed, 50 deletions(-)

diff --git a/http.c b/http.c
index 6e5f4ce5f9..5280511c74 100644
--- a/http.c
+++ b/http.c
@@ -22,9 +22,7 @@ static int min_curl_sessions = 1;
 static int curl_session_count;
 static int max_requests = -1;
 static CURLM *curlm;
-#ifndef NO_CURL_EASY_DUPHANDLE
 static CURL *curl_default;
-#endif
 
 #define PREV_BUF_SIZE 4096
 
@@ -382,24 +380,8 @@ static void init_curl_http_auth(CURL *result)
 
credential_fill(_auth);
 
-#if LIBCURL_VERSION_NUM >= 0x071301
curl_easy_setopt(result, CURLOPT_USERNAME, http_auth.username);
curl_easy_setopt(result, CURLOPT_PASSWORD, http_auth.password);
-#else
-   {
-   static struct strbuf up = STRBUF_INIT;
-   /*
-* Note that we assume we only ever have a single set of
-* credentials in a given program run, so we do not have
-* to worry about updating this buffer, only setting its
-* initial value.
-*/
-   if (!up.len)
-   strbuf_addf(, "%s:%s",
-   http_auth.username, http_auth.password);
-   curl_easy_setopt(result, CURLOPT_USERPWD, up.buf);
-   }
-#endif
 }
 
 /* *var must be free-able */
@@ -413,20 +395,10 @@ static void var_override(const char **var, char *value)
 
 static void set_proxyauth_name_password(CURL *result)
 {
-#if LIBCURL_VERSION_NUM >= 0x071301
curl_easy_setopt(result, CURLOPT_PROXYUSERNAME,
proxy_auth.username);
curl_easy_setopt(result, CURLOPT_PROXYPASSWORD,
proxy_auth.password);
-#else
-   struct strbuf s = STRBUF_INIT;
-
-   strbuf_addstr_urlencode(, proxy_auth.username, 1);
-   strbuf_addch(, ':');
-   strbuf_addstr_urlencode(, proxy_auth.password, 1);
-   curl_proxyuserpwd = strbuf_detach(, NULL);
-   curl_easy_setopt(result, CURLOPT_PROXYUSERPWD, 
curl_proxyuserpwd);
-#endif
 }
 
 static void init_curl_proxy_auth(CURL *result)
@@ -718,20 +690,12 @@ static CURL *get_curl_handle(void)
}
 
curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);
-#if LIBCURL_VERSION_NUM >= 0x071301
curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);
-#elif LIBCURL_VERSION_NUM >= 0x071101
curl_easy_setopt(result, CURLOPT_POST301, 1);
-#endif
-#if LIBCURL_VERSION_NUM >= 0x071304
curl_easy_setopt(result, CURLOPT_REDIR_PROTOCOLS,
 get_curl_allowed_protocols(0));
curl_easy_setopt(result, CURLOPT_PROTOCOLS,
 get_curl_allowed_protocols(-1));
-#else
-   warning("protocol restrictions not applied to curl redirects because\n"
-   "your curl version is too old (>= 7.19.4)");
-#endif
if (getenv("GIT_CURL_VERBOSE"))
curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
setup_curl_trace(result);
@@ -807,11 +771,9 @@ static CURL *get_curl_handle(void)
die("Invalid proxy URL '%s'", curl_http_proxy);
 
curl_easy_setopt(result, CURLOPT_PROXY, proxy_auth.host);
-#if LIBCURL_VERSION_NUM >= 0x071304
var_override(_no_proxy, getenv("NO_PROXY"));
var_override(_no_proxy, getenv("no_proxy"));
curl_easy_setopt(result, CURLOPT_NOPROXY, curl_no_proxy);
-#endif
}
init_curl_proxy_auth(result);
 
@@ -907,9 +869,7 @@ void http_init(struct remote *remote, const char *url, int 
proactive_auth)
ssl_cert_password_required = 1;
}
 
-#ifndef NO_CURL_EASY_DUPHANDLE
curl_default = get_curl_handle();
-#endif
 }
 
 void http_cleanup(void)
@@ -927,9 +887,7 @@ void http_cleanup(void)
}
active_queue_head = NULL;
 
-#ifndef NO_CURL_EASY_DUPHANDLE
curl_easy_cleanup(curl_default);
-#endif
 
curl_multi_cleanup(curlm);
curl_global_cleanup();
@@ -1003,11 +961,7 @@ struct active_request_slot *get_active_slot(void)
}
 
if (slot->curl == NULL) {
-#ifdef NO_CURL_EASY_DUPHANDLE
-   slot->curl = get_curl_handle();
-#else
slot->curl