From c181f490fc88a8a8fa1df283f4f0377b122aa482 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Tue, 6 Nov 2018 13:02:59 -0800
Subject: [PATCH] Enable HTTP/2 by default

This commit switches Cargo to using HTTP/2 by default. This is
controlled via the `http.multiplexing` configuration variable and has
been messaged out for testing [1] (although got very few responses).

There's been surprisingly little fallout from parallel downloads, so
let's see how this goes!

[1]: https://internals.rust-lang.org/t/testing-cargos-parallel-downloads/8466
---
 Cargo.toml                      |  4 ++--
 src/cargo/core/package.rs       | 35 +++++++++++++++++++++++++--------
 src/doc/src/reference/config.md |  2 +-
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 51e0627d3e0..11236d32874 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -22,8 +22,8 @@ bytesize = "1.0"
 crates-io = { path = "src/crates-io", version = "0.21" }
 crossbeam-utils = "0.5"
 crypto-hash = "0.3.1"
-curl = { version = "0.4.17", features = ['http2'] }
-curl-sys = "0.4.12"
+curl = { version = "0.4.19", features = ['http2'] }
+curl-sys = "0.4.15"
 env_logger = "0.5.11"
 failure = "0.1.2"
 filetime = "0.2"
diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs
index 171ac27e33e..560fee64ff1 100644
--- a/src/cargo/core/package.rs
+++ b/src/cargo/core/package.rs
@@ -7,10 +7,11 @@ use std::path::{Path, PathBuf};
 use std::time::{Instant, Duration};
 
 use bytesize::ByteSize;
-use curl;
-use curl_sys;
 use curl::easy::{Easy, HttpVersion};
 use curl::multi::{Multi, EasyHandle};
+use curl;
+use curl_sys;
+use failure::ResultExt;
 use lazycell::LazyCell;
 use semver::Version;
 use serde::ser;
@@ -333,7 +334,7 @@ impl<'cfg> PackageSet<'cfg> {
         // proxies.
         let mut multi = Multi::new();
         let multiplexing = config.get::<Option<bool>>("http.multiplexing")?
-            .unwrap_or(false);
+            .unwrap_or(true);
         multi.pipelining(false, multiplexing)
             .chain_err(|| "failed to enable multiplexing/pipelining in curl")?;
 
@@ -452,12 +453,30 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
         handle.follow_location(true)?; // follow redirects
 
         // Enable HTTP/2 to be used as it'll allow true multiplexing which makes
-        // downloads much faster. Currently Cargo requests the `http2` feature
-        // of the `curl` crate which means it should always be built in, so
-        // treat it as a fatal error of http/2 support isn't found.
+        // downloads much faster.
+        //
+        // Currently Cargo requests the `http2` feature of the `curl` crate
+        // which means it should always be built in. On OSX, however, we ship
+        // cargo still linked against the system libcurl. Building curl with
+        // ALPN support for HTTP/2 requires newer versions of OSX (the
+        // SecureTransport API) than we want to ship Cargo for. By linking Cargo
+        // against the system libcurl then older curl installations won't use
+        // HTTP/2 but newer ones will. All that to basically say we ignore
+        // errors here on OSX, but consider this a fatal error to not activate
+        // HTTP/2 on all other platforms.
         if self.set.multiplexing {
-            handle.http_version(HttpVersion::V2)
-                .chain_err(|| "failed to enable HTTP2, is curl not built right?")?;
+            let result = handle.http_version(HttpVersion::V2);
+            if cfg!(target_os = "macos") {
+                if let Err(e) = result {
+                    warn!("ignoring HTTP/2 activation error: {}", e)
+                }
+            } else {
+                result.with_context(|_| {
+                    "failed to enable HTTP2, is curl not built right?"
+                })?;
+            }
+        } else {
+            handle.http_version(HttpVersion::V11)?;
         }
 
         // This is an option to `libcurl` which indicates that if there's a
diff --git a/src/doc/src/reference/config.md b/src/doc/src/reference/config.md
index 8a22e95f922..da95970b28e 100644
--- a/src/doc/src/reference/config.md
+++ b/src/doc/src/reference/config.md
@@ -102,7 +102,7 @@ timeout = 30        # Timeout for each HTTP request, in seconds
 cainfo = "cert.pem" # Path to Certificate Authority (CA) bundle (optional)
 check-revoke = true # Indicates whether SSL certs are checked for revocation
 low-speed-limit = 5 # Lower threshold for bytes/sec (10 = default, 0 = disabled)
-multiplexing = false  # whether or not to use HTTP/2 multiplexing where possible
+multiplexing = true # whether or not to use HTTP/2 multiplexing where possible
 
 # This setting can be used to help debug what's going on with HTTP requests made
 # by Cargo. When set to `true` then Cargo's normal debug logging will be filled