From 44fac8934a3b92cc4bfb87dead31f90aa75683ee Mon Sep 17 00:00:00 2001 From: binarycat Date: Sun, 25 Aug 2024 12:09:58 -0400 Subject: [PATCH 1/2] explain the options curl passes to bootstrap also fixes a discrepency where the rust side doesn't use -L must not be merged before #129134 docs are only on the rust side, since duplicated prose has a tendancy to get out-of-sync, and also because there are talks of removing the python script all together eventually. --- src/bootstrap/bootstrap.py | 4 ++++ src/bootstrap/src/core/download.rs | 19 +++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index c19134b4594..1de010dc08a 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -115,6 +115,10 @@ def _download(path, url, probably_big, verbose, exception): extra_flags = [] if curl_version() > (7, 70): extra_flags = [ "--retry-all-errors" ] + # options should be kept in sync with + # src/bootstrap/src/core/download.rs + # for consistency. + # they are also more compreprensivly explained in that file. run(["curl", option] + extra_flags + [ "-L", # Follow redirect. "-y", "30", "-Y", "10", # timeout if speed is < 10 bytes/sec for > 30 seconds diff --git a/src/bootstrap/src/core/download.rs b/src/bootstrap/src/core/download.rs index b5c55854eff..038ca320e6b 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -227,20 +227,35 @@ fn download_file(&self, url: &str, dest_path: &Path, help_on_error: &str) { fn download_http_with_retries(&self, tempfile: &Path, url: &str, help_on_error: &str) { println!("downloading {url}"); // Try curl. If that fails and we are on windows, fallback to PowerShell. + // options should be kept in sync with + // src/bootstrap/src/core/download.rs + // for consistency let mut curl = command("curl"); curl.args([ + // follow redirect + "-L", + // timeout if speed is < 10 bytes/sec for > 30 seconds "-y", "30", "-Y", - "10", // timeout if speed is < 10 bytes/sec for > 30 seconds + "10", + // timeout if cannot connect within 30 seconds "--connect-timeout", - "30", // timeout if cannot connect within 30 seconds + "30", + // output file "-o", tempfile.to_str().unwrap(), + // if there is an error, don't restart the download, + // instead continue where it left off. "--continue-at", "-", + // retry up to 3 times. note that this means a maximum of 4 + // attempts will be made, since the first attempt isn't a *re*try. "--retry", "3", + // -S: show errors, even if -s is specified + // -R: set timestamp of downloaded file to that of the server + // -f: fail on non-ok http status "-SRf", ]); // Don't print progress in CI; the \r wrapping looks bad and downloads don't take long enough for progress to be useful. From 757affd1a9147f4ccc0481df6037a867e0a21ad7 Mon Sep 17 00:00:00 2001 From: binarycat Date: Thu, 5 Sep 2024 15:12:15 -0400 Subject: [PATCH 2/2] bootstrap: pass long options to curl --- src/bootstrap/bootstrap.py | 17 ++++++++++------- src/bootstrap/src/core/download.rs | 20 +++++++++++--------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 1de010dc08a..f1d45c880ee 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -106,9 +106,9 @@ def _download(path, url, probably_big, verbose, exception): try: if (probably_big or verbose) and "GITHUB_ACTIONS" not in os.environ: - option = "-#" + option = "--progress-bar" else: - option = "-s" + option = "--silent" # 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()) @@ -120,12 +120,15 @@ def _download(path, url, probably_big, verbose, exception): # for consistency. # they are also more compreprensivly explained in that file. 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, + # Follow redirect. + "--location", + # timeout if speed is < 10 bytes/sec for > 30 seconds + "--speed-time", "30", "--speed-limit", "10", + # timeout if cannot connect within 30 seconds + "--connect-timeout", "30", + "--output", path, "--continue-at", "-", - "--retry", "3", "-SRf", url], + "--retry", "3", "--show-error", "--remote-time", "--fail", 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 038ca320e6b..77a4f4b0f33 100644 --- a/src/bootstrap/src/core/download.rs +++ b/src/bootstrap/src/core/download.rs @@ -233,17 +233,17 @@ fn download_http_with_retries(&self, tempfile: &Path, url: &str, help_on_error: let mut curl = command("curl"); curl.args([ // follow redirect - "-L", + "--location", // timeout if speed is < 10 bytes/sec for > 30 seconds - "-y", + "--speed-time", "30", - "-Y", + "--speed-limit", "10", // timeout if cannot connect within 30 seconds "--connect-timeout", "30", // output file - "-o", + "--output", tempfile.to_str().unwrap(), // if there is an error, don't restart the download, // instead continue where it left off. @@ -253,14 +253,16 @@ fn download_http_with_retries(&self, tempfile: &Path, url: &str, help_on_error: // attempts will be made, since the first attempt isn't a *re*try. "--retry", "3", - // -S: show errors, even if -s is specified - // -R: set timestamp of downloaded file to that of the server - // -f: fail on non-ok http status - "-SRf", + // show errors, even if --silent is specified + "--show-error", + // set timestamp of downloaded file to that of the server + "--remote-time", + // fail on non-ok http status + "--fail", ]); // Don't print progress in CI; the \r wrapping looks bad and downloads don't take long enough for progress to be useful. if CiEnv::is_ci() { - curl.arg("-s"); + curl.arg("--silent"); } else { curl.arg("--progress-bar"); }