Thread (99 messages) flat view 99 messages, 7 authors, 2016-12-15

Re: [PATCH v9 3/5] http: always warn if libcurl version is too old

From: Brandon Williams <hidden>
Date: 2016-12-14 17:56:43

On 12/14, Jeff King wrote:
On Tue, Dec 13, 2016 at 05:40:35PM -0800, Brandon Williams wrote:
quoted
diff --git a/transport.c b/transport.c
index e1ba78b..fbd799d 100644
--- a/transport.c
+++ b/transport.c
@@ -700,11 +700,6 @@ void transport_check_allowed(const char *type)
 		die("transport '%s' not allowed", type);
 }
 
-int transport_restrict_protocols(void)
-{
-	return !!protocol_whitelist();
-}
-
This function was subtly broken as of patch 2 of the series. It's
probably not a big deal in the long run, but should the series be
re-ordered to put this one first?

I think the commit message would need adjusted, but it probably should
mention the reasons this is a good idea even _without_ the new config
system. Namely that even when there's no protocol whitelist, newer
versions of curl have all of the other non-http protocols disabled.

I wonder if anybody is actually using a version of curl old enough to
trigger this. If so, they're going to get the warning every time they
fetch via http. We might need to stick it behind an "advice.*" config
option, though I'm inclined to leave it as-is and see if anybody
actually complains.

-Peff
Yeah you're right, transport_restrict_protocols() is definitely broken
after patch 2.  Since I'm probably going to need to do a reroll based on
some of your comments in the other patches in the series we might as
well reorder patch 2 and 3 so this isn't broken between patches.

-- 
Brandon Williams
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help