From a26a2d71a8aa6e3a8651ad5ab04b0f07334fa298 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Thu, 9 Feb 2023 18:01:41 +0000 Subject: [PATCH 1/3] Use --speed-time over --max-time for API downloads --- Library/Homebrew/api.rb | 13 +++++++------ Library/Homebrew/cmd/update.sh | 9 +++++++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index 3abe1310a1..9d84788701 100644 --- a/Library/Homebrew/api.rb +++ b/Library/Homebrew/api.rb @@ -19,8 +19,10 @@ module Homebrew HOMEBREW_CACHE_API = (HOMEBREW_CACHE/"api").freeze - # Set a longer timeout just for large(r) files. - JSON_API_MAX_TIME = 30 + # Timeout values to check for dead connections + # We don't use --max-time to support slow connections + JSON_API_SPEED_MARGIN = 100 # bytes/sec + JSON_API_SPEED_TIME = 10 # seconds of downloading under the margin sig { params(endpoint: String).returns(Hash) } def fetch(endpoint) @@ -45,15 +47,14 @@ module Homebrew retry_count = 0 url = "#{Homebrew::EnvConfig.api_domain}/#{endpoint}" default_url = "#{HOMEBREW_API_DEFAULT_DOMAIN}/#{endpoint}" - curl_args = %w[--compressed --silent] + curl_args = %W[--compressed --speed-limit #{JSON_API_SPEED_MARGIN} --speed-time #{JSON_API_SPEED_TIME}] + curl_args.prepend("--silent") unless Context.current.debug? curl_args.prepend("--time-cond", target) if target.exist? && !target.empty? begin begin # Disable retries here, we handle them ourselves below. - Utils::Curl.curl_download(*curl_args, url, to: target, - max_time: JSON_API_MAX_TIME, retries: 0, - show_error: false) + Utils::Curl.curl_download(*curl_args, url, to: target, retries: 0, show_error: false) rescue ErrorDuringExecution if url == default_url raise unless target.exist? diff --git a/Library/Homebrew/cmd/update.sh b/Library/Homebrew/cmd/update.sh index 0eb9a28ddc..476b057abc 100644 --- a/Library/Homebrew/cmd/update.sh +++ b/Library/Homebrew/cmd/update.sh @@ -785,11 +785,16 @@ EOS JSON_URLS+=("${HOMEBREW_API_DEFAULT_DOMAIN}/${formula_or_cask}.json") for json_url in "${JSON_URLS[@]}" do + time_cond=() + if [[ -s "${HOMEBREW_CACHE}/api/${formula_or_cask}.json" ]] + then + time_cond=("--time-cond" "${HOMEBREW_CACHE}/api/${formula_or_cask}.json") + fi curl \ "${CURL_DISABLE_CURLRC_ARGS[@]}" \ - --fail --compressed --silent --max-time 30 \ + --fail --compressed --silent --speed-limit 100 --speed-time 30 \ --location --remote-time --output "${HOMEBREW_CACHE}/api/${formula_or_cask}.json" \ - --time-cond "${HOMEBREW_CACHE}/api/${formula_or_cask}.json" \ + "${time_cond[@]}" \ --user-agent "${HOMEBREW_USER_AGENT_CURL}" \ "${json_url}" curl_exit_code=$? From 15c291f054df961dc4a6731dfb6aea9b121b9d85 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Thu, 9 Feb 2023 18:26:37 +0000 Subject: [PATCH 2/3] api: don't add --time-cond on retries --- Library/Homebrew/api.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index 9d84788701..8f8d304ad6 100644 --- a/Library/Homebrew/api.rb +++ b/Library/Homebrew/api.rb @@ -49,12 +49,13 @@ module Homebrew default_url = "#{HOMEBREW_API_DEFAULT_DOMAIN}/#{endpoint}" curl_args = %W[--compressed --speed-limit #{JSON_API_SPEED_MARGIN} --speed-time #{JSON_API_SPEED_TIME}] curl_args.prepend("--silent") unless Context.current.debug? - curl_args.prepend("--time-cond", target) if target.exist? && !target.empty? begin begin + args = curl_args.dup + args.prepend("--time-cond", target) if target.exist? && !target.empty? # Disable retries here, we handle them ourselves below. - Utils::Curl.curl_download(*curl_args, url, to: target, retries: 0, show_error: false) + Utils::Curl.curl_download(*args, url, to: target, retries: 0, show_error: false) rescue ErrorDuringExecution if url == default_url raise unless target.exist? From 51d63ee484896dbced866c764360edea035355c2 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Thu, 9 Feb 2023 18:27:37 +0000 Subject: [PATCH 3/3] cmd/update.sh: fix API download reporting --- Library/Homebrew/cmd/update.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/cmd/update.sh b/Library/Homebrew/cmd/update.sh index 476b057abc..b5928ea6fd 100644 --- a/Library/Homebrew/cmd/update.sh +++ b/Library/Homebrew/cmd/update.sh @@ -709,13 +709,6 @@ EOS wait trap - SIGINT - if [[ -f "${update_failed_file}" ]] - then - onoe <"${update_failed_file}" - rm -f "${update_failed_file}" - export HOMEBREW_UPDATE_FAILED="1" - fi - if [[ -f "${missing_remote_ref_dirs_file}" ]] then HOMEBREW_MISSING_REMOTE_REF_DIRS="$(cat "${missing_remote_ref_dirs_file}")" @@ -813,6 +806,13 @@ EOS done fi + if [[ -f "${update_failed_file}" ]] + then + onoe <"${update_failed_file}" + rm -f "${update_failed_file}" + export HOMEBREW_UPDATE_FAILED="1" + fi + safe_cd "${HOMEBREW_REPOSITORY}" # HOMEBREW_UPDATE_AUTO wasn't modified in subshell.