diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 6a069bdef9..74562d1ca3 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -1,18 +1,11 @@ # frozen_string_literal: true +require "download_strategy" require "cli/parser" require "utils/github" require "tmpdir" require "bintray" -class CurlNoResumeDownloadStrategy < CurlDownloadStrategy - private - - def _fetch(url:, resolved_url:) - curl("--location", "--remote-time", "--create-dirs", "--output", temporary_path, resolved_url) - end -end - module Homebrew module_function @@ -148,6 +141,27 @@ module Homebrew nil end + def download_artifact(url, dir, pr) + token, username = GitHub.api_credentials + case GitHub.api_credentials_type + when :env_username_password, :keychain_username_password + curl_args = ["--user", "#{username}:#{token}"] + when :env_token + curl_args = ["--header", "Authorization: token #{token}"] + when :none + raise Error, "Credentials must be set to access the Artifacts API" + end + + # Download the artifact as a zip file and unpack it into `dir`. This is + # preferred over system `curl` and `tar` as this leverages the Homebrew + # cache to avoid repeated downloads of (possibly large) bottles. + FileUtils.chdir dir do + downloader = GitHubArtifactDownloadStrategy.new(url, "artifact", pr, curl_args: curl_args, secrets: [token]) + downloader.fetch + downloader.stage + end + end + def pr_pull pr_pull_args.parse @@ -163,7 +177,7 @@ module Homebrew workflow = args.workflow || "tests.yml" artifact = args.artifact || "bottles" - tap = Tap.fetch(args.tap || "homebrew/core") + tap = Tap.fetch(args.tap || CoreTap.instance.name) setup_git_environment! @@ -187,9 +201,8 @@ module Homebrew next end - GitHub.fetch_artifact(user, repo, pr, dir, workflow_id: workflow, - artifact_name: artifact, - strategy: CurlNoResumeDownloadStrategy) + url = GitHub.get_artifact_url(user, repo, pr, workflow_id: workflow, artifact_name: artifact) + download_artifact(url, dir, pr) if Homebrew.args.dry_run? puts "brew bottle --merge --write #{Dir["*.json"].join " "}" @@ -209,3 +222,32 @@ module Homebrew end end end + +class GitHubArtifactDownloadStrategy < AbstractFileDownloadStrategy + def fetch + ohai "Downloading #{url}" + if cached_location.exist? + puts "Already downloaded: #{cached_location}" + else + begin + curl "--location", "--create-dirs", "--output", temporary_path, url, + *meta.fetch(:curl_args, []), + secrets: meta.fetch(:secrets, []) + rescue ErrorDuringExecution + raise CurlDownloadStrategyError, url + end + ignore_interrupts do + cached_location.dirname.mkpath + temporary_path.rename(cached_location) + symlink_location.dirname.mkpath + end + end + FileUtils.ln_s cached_location.relative_path_from(symlink_location.dirname), symlink_location, force: true + end + + private + + def resolved_basename + "artifact.zip" + end +end diff --git a/Library/Homebrew/test/utils/github_spec.rb b/Library/Homebrew/test/utils/github_spec.rb index c2dc27b443..939ac6de5b 100644 --- a/Library/Homebrew/test/utils/github_spec.rb +++ b/Library/Homebrew/test/utils/github_spec.rb @@ -42,17 +42,22 @@ describe GitHub do end end - describe "::fetch_artifact", :needs_network do + describe "::get_artifact_url", :needs_network do it "fails to find a nonexistant workflow" do expect { - subject.fetch_artifact("Homebrew", "homebrew-core", 1, ".") + subject.get_artifact_url("Homebrew", "homebrew-core", 1) }.to raise_error(/No matching workflow run found/) end it "fails to find artifacts that don't exist" do expect { - subject.fetch_artifact("Homebrew", "homebrew-core", 51971, ".", artifact_name: "false_bottles") + subject.get_artifact_url("Homebrew", "homebrew-core", 51971, artifact_name: "false_bottles") }.to raise_error(/No artifact .+ was found/) end + + it "gets an artifact link" do + url = subject.get_artifact_url("Homebrew", "homebrew-core", 51971, artifact_name: "bottles") + expect(url).to eq("https://api.github.com/repos/Homebrew/homebrew-core/actions/artifacts/3557392/zip") + end end end diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index c017a63b37..2cad7fad04 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "download_strategy" require "tempfile" require "uri" @@ -435,8 +434,7 @@ module GitHub scopes: CREATE_ISSUE_FORK_OR_PR_SCOPES) end - def fetch_artifact(user, repo, pr, dir, - workflow_id: "tests.yml", artifact_name: "bottles", strategy: CurlDownloadStrategy) + def get_artifact_url(user, repo, pr, workflow_id: "tests.yml", artifact_name: "bottles") scopes = CREATE_ISSUE_FORK_OR_PR_SCOPES base_url = "#{API_URL}/repos/#{user}/#{repo}" pr_payload = open_api("#{base_url}/pulls/#{pr}", scopes: scopes) @@ -479,28 +477,7 @@ module GitHub EOS end - artifact_url = artifact.first["archive_download_url"] - - token, username = api_credentials - case api_credentials_type - when :env_username_password, :keychain_username_password - curl_args = { user: "#{username}:#{token}" } - when :env_token - curl_args = { header: "Authorization: token #{token}" } - when :none - raise Error, "Credentials must be set to access the Artifacts API" - end - - # Download the artifact as a zip file and unpack it into `dir`. This is - # preferred over system `curl` and `tar` as this leverages the Homebrew - # cache to avoid repeated downloads of (possibly large) bottles. - FileUtils.chdir dir do - curl_args[:cache] = Pathname.new(dir) - curl_args[:secrets] = [token] - downloader = strategy.new(artifact_url, "artifact", pr, **curl_args) - downloader.fetch - downloader.stage - end + artifact.first["archive_download_url"] end def api_errors