From 12e638978814d84e4e09b172f784d5f16759d8e1 Mon Sep 17 00:00:00 2001 From: binarycat Date: Thu, 15 Aug 2024 16:22:01 -0400 Subject: [PATCH 1/5] bootstrap: improve error recovery flags to curl alternative to #128459 fixes #110178 --- src/bootstrap/bootstrap.py | 8 +++++++- src/bootstrap/src/core/download.rs | 31 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 4e8e0fd2532f1..5ea4b4882a975 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -79,6 +79,8 @@ def get(base, url, path, checksums, verbose=False): eprint("removing", temp_path) os.unlink(temp_path) +def curl_version(): + return float(re.match(bytes("^curl ([0-9]+\\.[0-9]+)", "utf8"), require(["curl", "-V"]))[1]) def download(path, url, probably_big, verbose): for _ in range(4): @@ -107,11 +109,15 @@ def _download(path, url, probably_big, verbose, exception): # If curl is not present on Win32, we should not sys.exit # but raise `CalledProcessError` or `OSError` instead require(["curl", "--version"], exception=platform_is_win32()) - run(["curl", option, + extra_flags = [] + if curl_version() > 7.70: + extra_flags = [ "--retry-all-errors" ] + run(["curl", option] + extra_flags + [ "-L", # Follow redirect. "-y", "30", "-Y", "10", # timeout if speed is < 10 bytes/sec for > 30 seconds "--connect-timeout", "30", # timeout if cannot connect within 30 seconds "-o", path, + "--continue-at", "-", "--retry", "3", "-SRf", url], verbose=verbose, exception=True, # Will raise RuntimeError on failure diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 4d1aea3cd956a..ed7637e8ee097 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -21,6 +21,31 @@ fn try_run(config: &Config, cmd: &mut Command) -> Result<(), ()> { config.try_run(cmd) } +fn extract_curl_version(out: &[u8]) -> f32 { + let out = &out[5..]; + let Some(i) = out.iter().position(|&x| x == b' ') else { return 0.0 }; + let out = &out[..i]; + let Some(k) = out.iter().rev().position(|&x| x == b'.') else { return 0.0 }; + let out = &out[..out.len()-k-1]; + std::str::from_utf8(out).unwrap().parse().unwrap_or(0.0) +} + +#[test] +fn test_extract_curl_version() { + assert_eq!(extract_curl_version(b"\ + curl 8.4.0 (x86_64-pc-linux-gnu) libcurl/8.4.0 \ + OpenSSL/3.0.13 zlib/1.3 brotli/1.1.0 zstd/1.5.5 libidn2/2.3.4 \ + libssh2/1.11.0 nghttp2/1.57.0"), 8.4); +} + +fn curl_version() -> f32 { + let mut curl = Command::new("curl"); + curl.arg("-V"); + let Ok(out) = curl.output() else { return 0.0 }; + let out = out.stdout; + extract_curl_version(&out) +} + /// Generic helpers that are useful anywhere in bootstrap. impl Config { pub fn is_verbose(&self) -> bool { @@ -219,6 +244,8 @@ impl Config { "30", // timeout if cannot connect within 30 seconds "-o", tempfile.to_str().unwrap(), + "--continue-at", + "-", "--retry", "3", "-SRf", @@ -229,6 +256,10 @@ impl Config { } else { curl.arg("--progress-bar"); } + // --retry-all-errors was added in 7.71.0, don't use it if curl is old. + if dbg!(curl_version()) > 7.70 { + curl.arg("--retry-all-errors"); + } curl.arg(url); if !self.check_run(&mut curl) { if self.build.contains("windows-msvc") { From b3ec296f3d918efdbe4db02dbd7fbc7fea1641a1 Mon Sep 17 00:00:00 2001 From: binarycat Date: Fri, 16 Aug 2024 23:25:18 -0400 Subject: [PATCH 2/5] autoformat and remove unit test --- src/bootstrap/src/core/download.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index ed7637e8ee097..2bde05501fcfc 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -26,18 +26,10 @@ fn extract_curl_version(out: &[u8]) -> f32 { let Some(i) = out.iter().position(|&x| x == b' ') else { return 0.0 }; let out = &out[..i]; let Some(k) = out.iter().rev().position(|&x| x == b'.') else { return 0.0 }; - let out = &out[..out.len()-k-1]; + let out = &out[..out.len() - k - 1]; std::str::from_utf8(out).unwrap().parse().unwrap_or(0.0) } -#[test] -fn test_extract_curl_version() { - assert_eq!(extract_curl_version(b"\ - curl 8.4.0 (x86_64-pc-linux-gnu) libcurl/8.4.0 \ - OpenSSL/3.0.13 zlib/1.3 brotli/1.1.0 zstd/1.5.5 libidn2/2.3.4 \ - libssh2/1.11.0 nghttp2/1.57.0"), 8.4); -} - fn curl_version() -> f32 { let mut curl = Command::new("curl"); curl.arg("-V"); From d1b41f3747ecb01b717bd50918513227de36f3cb Mon Sep 17 00:00:00 2001 From: binarycat Date: Thu, 22 Aug 2024 19:04:34 -0400 Subject: [PATCH 3/5] remove dbg!() --- src/bootstrap/src/core/download.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 2bde05501fcfc..201addaec2fc5 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -249,7 +249,7 @@ impl Config { curl.arg("--progress-bar"); } // --retry-all-errors was added in 7.71.0, don't use it if curl is old. - if dbg!(curl_version()) > 7.70 { + if curl_version() > 7.70 { curl.arg("--retry-all-errors"); } curl.arg(url); From 69ca95bf7f5462eb7ade2de18d1f0d79b2c21cfd Mon Sep 17 00:00:00 2001 From: binarycat Date: Fri, 23 Aug 2024 11:48:04 -0400 Subject: [PATCH 4/5] use tuples for semver, not floats --- src/bootstrap/bootstrap.py | 7 +++++-- src/bootstrap/src/core/download.rs | 18 +++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 5ea4b4882a975..c19134b4594fb 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -80,7 +80,10 @@ def get(base, url, path, checksums, verbose=False): os.unlink(temp_path) def curl_version(): - return float(re.match(bytes("^curl ([0-9]+\\.[0-9]+)", "utf8"), require(["curl", "-V"]))[1]) + m = re.match(bytes("^curl ([0-9]+)\\.([0-9]+)", "utf8"), require(["curl", "-V"])) + if m is None: + return (0, 0) + return (int(m[1]), int(m[2])) def download(path, url, probably_big, verbose): for _ in range(4): @@ -110,7 +113,7 @@ def _download(path, url, probably_big, verbose, exception): # but raise `CalledProcessError` or `OSError` instead require(["curl", "--version"], exception=platform_is_win32()) extra_flags = [] - if curl_version() > 7.70: + if curl_version() > (7, 70): extra_flags = [ "--retry-all-errors" ] run(["curl", option] + extra_flags + [ "-L", # Follow redirect. diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index 201addaec2fc5..fd6f17130ee91 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -21,19 +21,23 @@ fn try_run(config: &Config, cmd: &mut Command) -> Result<(), ()> { config.try_run(cmd) } -fn extract_curl_version(out: &[u8]) -> f32 { +fn extract_curl_version(out: &[u8]) -> (u16, u16) { let out = &out[5..]; - let Some(i) = out.iter().position(|&x| x == b' ') else { return 0.0 }; + let Some(i) = out.iter().position(|&x| x == b' ') else { return (0, 0) }; let out = &out[..i]; - let Some(k) = out.iter().rev().position(|&x| x == b'.') else { return 0.0 }; + let Some(k) = out.iter().rev().position(|&x| x == b'.') else { return (0, 0) }; let out = &out[..out.len() - k - 1]; - std::str::from_utf8(out).unwrap().parse().unwrap_or(0.0) + let Ok(s) = std::str::from_utf8(out) else { return (0, 0) }; + let parts = s.split('.').collect::>(); + let [s_major, s_minor] = &parts[..] else { return (0, 0) }; + let (Ok(major), Ok(minor)) = (s_major.parse(), s_minor.parse()) else { return (0, 0) }; + (major, minor) } -fn curl_version() -> f32 { +fn curl_version() -> (u16, u16) { let mut curl = Command::new("curl"); curl.arg("-V"); - let Ok(out) = curl.output() else { return 0.0 }; + let Ok(out) = curl.output() else { return (0, 0) }; let out = out.stdout; extract_curl_version(&out) } @@ -249,7 +253,7 @@ impl Config { curl.arg("--progress-bar"); } // --retry-all-errors was added in 7.71.0, don't use it if curl is old. - if curl_version() > 7.70 { + if curl_version() > (7, 70) { curl.arg("--retry-all-errors"); } curl.arg(url); From 56adf87213b22ef848fb8afd3b8180c0455456d7 Mon Sep 17 00:00:00 2001 From: binarycat Date: Sat, 24 Aug 2024 12:41:40 -0400 Subject: [PATCH 5/5] rewrite extract_curl_version again --- src/bootstrap/src/core/download.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index fd6f17130ee91..b5c55854eff58 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -21,23 +21,20 @@ fn try_run(config: &Config, cmd: &mut Command) -> Result<(), ()> { config.try_run(cmd) } -fn extract_curl_version(out: &[u8]) -> (u16, u16) { - let out = &out[5..]; - let Some(i) = out.iter().position(|&x| x == b' ') else { return (0, 0) }; - let out = &out[..i]; - let Some(k) = out.iter().rev().position(|&x| x == b'.') else { return (0, 0) }; - let out = &out[..out.len() - k - 1]; - let Ok(s) = std::str::from_utf8(out) else { return (0, 0) }; - let parts = s.split('.').collect::>(); - let [s_major, s_minor] = &parts[..] else { return (0, 0) }; - let (Ok(major), Ok(minor)) = (s_major.parse(), s_minor.parse()) else { return (0, 0) }; - (major, minor) +fn extract_curl_version(out: &[u8]) -> semver::Version { + let out = String::from_utf8_lossy(out); + // The output should look like this: "curl .. ..." + out.lines() + .next() + .and_then(|line| line.split(" ").nth(1)) + .and_then(|version| semver::Version::parse(version).ok()) + .unwrap_or(semver::Version::new(1, 0, 0)) } -fn curl_version() -> (u16, u16) { +fn curl_version() -> semver::Version { let mut curl = Command::new("curl"); curl.arg("-V"); - let Ok(out) = curl.output() else { return (0, 0) }; + let Ok(out) = curl.output() else { return semver::Version::new(1, 0, 0) }; let out = out.stdout; extract_curl_version(&out) } @@ -253,7 +250,7 @@ impl Config { curl.arg("--progress-bar"); } // --retry-all-errors was added in 7.71.0, don't use it if curl is old. - if curl_version() > (7, 70) { + if curl_version() >= semver::Version::new(7, 71, 0) { curl.arg("--retry-all-errors"); } curl.arg(url);