From 31a152208bae56880119cbeee2ace8943c3f6e5e Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Wed, 3 May 2023 16:16:32 +0800 Subject: [PATCH 1/2] github_packages: improve upload error handling Erroring out in the middle of uploading multiple bottles results in a state that is tedious to recover from. Let's try to avoid these situations by performing checks for all the bottles first before trying to upload any. --- Library/Homebrew/github_packages.rb | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/github_packages.rb b/Library/Homebrew/github_packages.rb index 636eb0823c..ca61feaaea 100644 --- a/Library/Homebrew/github_packages.rb +++ b/Library/Homebrew/github_packages.rb @@ -57,9 +57,20 @@ class GitHubPackages load_schemas! bottles_hash.each do |formula_full_name, bottle_hash| + # First, check that we won't encounter an error in the middle of uploading bottles. + preupload_check(user, token, skopeo, formula_full_name, bottle_hash, + keep_old: keep_old, dry_run: dry_run, warn_on_error: warn_on_error) + end + + # We intentionally iterate over `bottles_hash` twice to + # avoid erroring out in the middle of uploading bottles. + # rubocop:disable Style/CombinableLoops + bottles_hash.each do |formula_full_name, bottle_hash| + # Next, upload the bottles after checking them all. upload_bottle(user, token, skopeo, formula_full_name, bottle_hash, keep_old: keep_old, dry_run: dry_run, warn_on_error: warn_on_error) end + # rubocop:enable Style/CombinableLoops end def self.version_rebuild(version, rebuild, bottle_tag = nil) @@ -191,7 +202,7 @@ class GitHubPackages end end - def upload_bottle(user, token, skopeo, formula_full_name, bottle_hash, keep_old:, dry_run:, warn_on_error:) + def preupload_check(user, token, skopeo, _formula_full_name, bottle_hash, keep_old:, dry_run:, warn_on_error:) formula_name = bottle_hash["formula"]["name"] _, org, repo, = *bottle_hash["bottle"]["root_url"].match(URL_REGEX) @@ -237,6 +248,16 @@ class GitHubPackages end end + [formula_name, org, repo, version, rebuild, version_rebuild, image_name, image_uri, keep_old] + end + + def upload_bottle(user, token, skopeo, formula_full_name, bottle_hash, keep_old:, dry_run:, warn_on_error:) + # We run the preupload check twice to prevent TOCTOU bugs. + result = preupload_check(user, token, skopeo, formula_full_name, bottle_hash, + keep_old: keep_old, dry_run: dry_run, warn_on_error: warn_on_error) + + formula_name, org, repo, version, rebuild, version_rebuild, image_name, image_uri, keep_old = *result + root = Pathname("#{formula_name}--#{version_rebuild}") FileUtils.rm_rf root root.mkpath From 9744b690717bdc4517e35c3734868fa567504351 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Wed, 3 May 2023 16:53:28 +0800 Subject: [PATCH 2/2] github_packages: use exponential backoff when retrying The retry behaviour in `publish_commit_bottles.yml` [1] is often successful after the second try, so it's likely that we're not waiting long enough in between retries here. Let's fix that by retrying with exponential backoff instead of adding a fixed interval of five seconds after each failure. [1] https://github.com/Homebrew/homebrew-core/blob/3241035b2a61866995bbad3264d354d5c30644dc/.github/workflows/publish-commit-bottles.yml#L431-L443 --- Library/Homebrew/github_packages.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/github_packages.rb b/Library/Homebrew/github_packages.rb index ca61feaaea..602ba38d9b 100644 --- a/Library/Homebrew/github_packages.rb +++ b/Library/Homebrew/github_packages.rb @@ -440,7 +440,7 @@ class GitHubPackages rescue ErrorDuringExecution retry_count += 1 odie "Cannot perform an upload to registry after retrying multiple times!" if retry_count >= 5 - sleep 5*retry_count + sleep 5 ** retry_count retry end