Re: [PATCH 0/2] http: handle curl with vendor backports

Jeff King [off-list ref] writes:

On Fri, Aug 11, 2017 at 03:15:06PM -0700, Junio C Hamano wrote:

quoted

"Tom G. Christensen" [off-list ref] writes:

quoted

The curl packages provided by Red Hat for RHEL contain several
backports of features from later curl releases.
This causes problems with current version based checks in http.c.

Here is an overview of the features that have been backported:
7.10.6 (el3) Backports CURLPROTO_*
7.12.1 (el4) Backports CURLPROTO_*
7.15.5 (el5) Backports GSSAPI_DELEGATION_*
             Backports CURLPROTO_*
7.19.7 (el6) Backports GSSAPI_DELEGATION_*
             Backports CURL_SSL_VERSION_TLSv1_{0,1,2}
7.29.0 (el7) Backports CURL_SSL_VERSION_TLSv1_{0,1,2}

This patch series will update the current version based checks for
protocol restriction and GSSAPI delegation control support to ones
based on features to properly deal with the above listed backports.
The fine grained TLS version support does not seem to be
distinguishable via a preprocessor macro so I've left that alone.

Thanks; these feature macros ought to be more dependable, and I
think this moves things in the right direction (regardless of which
features we might later pick as mandatory and cut off supports for
older versions).

Yes, I agree that these are an improvement regardless. If we follow
through on the cut-off to 7.19.4, then the CURLPROTO ones all go away.
But I don't mind rebasing any cut-off proposal on top of this work.

Yeah I came to a similar conclusion and was about asking if you feel
the same way that your series should be made on top of Tom's fixes.

The aspect of that series I do like the most is to base our
decisions on features, not versions, and I also wonder if we can do
similar in your "abandon too old ones" series, too.

Thanks.

On Sun, Aug 20, 2017 at 09:28:20AM -0700, Junio C Hamano wrote:

quoted

Yes, I agree that these are an improvement regardless. If we follow
through on the cut-off to 7.19.4, then the CURLPROTO ones all go away.
But I don't mind rebasing any cut-off proposal on top of this work.

Yeah I came to a similar conclusion and was about asking if you feel
the same way that your series should be made on top of Tom's fixes.

The aspect of that series I do like the most is to base our
decisions on features, not versions, and I also wonder if we can do
similar in your "abandon too old ones" series, too.

Yeah, I don't mind moving to feature flags where we can (though some
features do not have a useful flag; e.g., the only way to know whether
we must be strdup curl_easy_setopt() arguments is by checking the curl
version).

One annoying thing about "feature" flags instead of version flags is
that it takes a lot of legwork to figure out how old those features are
(whereas with the versions I was able to look that up in the curl
history pretty easily).  Since people adding the feature flag generally
do that legwork, it's probably worth having a comment for each
mentioning the general vintage (or maybe the commit message is an OK
place for that).

I actually wonder if it is worth defining our own readable flags in a
big table at the beginning of the file, like:

  /*
   * introduced in curl 7.19.4, but backported by some distros like
   * RHEL. We can identify it by the presence of the PROTO flags.
   */
  #ifdef CURLPROTO_HTTP
  #define CURL_SUPPORTS_PROTOCOL_REDIRECTION
  #endif

That keeps the logic in one place (where it can be changed if we later
find that the define we picked for our feature isn't quite accurate).
And then the #ifdefs sprinkled through the code itself become
self-documenting.

-Peff