From 3f00d08544fb97aa08747280800ef08c489b88c4 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Wed, 10 Mar 2021 16:08:45 +0000 Subject: [PATCH] dev-cmd/pr-upload: some refactoring - make the description more generic/correct - use "internet archive" over "archive" - move some logic to a new `GitHubReleases` class (for consistency) - remove some obvious comments - extract out and move some constants --- Library/Homebrew/archive.rb | 13 +++--- Library/Homebrew/bintray.rb | 1 + Library/Homebrew/dev-cmd/pr-upload.rb | 45 +++++-------------- Library/Homebrew/github_releases.rb | 44 ++++++++++++++++++ Library/Homebrew/global.rb | 2 - .../sorbet/rbi/hidden-definitions/hidden.rbi | 1 - completions/fish/brew.fish | 2 +- completions/zsh/_brew | 2 +- docs/Manpage.md | 2 +- manpages/brew.1 | 2 +- 10 files changed, 68 insertions(+), 46 deletions(-) create mode 100644 Library/Homebrew/github_releases.rb diff --git a/Library/Homebrew/archive.rb b/Library/Homebrew/archive.rb index 3e43dcc272..46e4e6a64b 100644 --- a/Library/Homebrew/archive.rb +++ b/Library/Homebrew/archive.rb @@ -16,6 +16,9 @@ class Archive class Error < RuntimeError end + URL_PREFIX = "https://archive.org" + S3_DOMAIN = "s3.us.archive.org" + sig { returns(String) } def inspect "#" @@ -34,7 +37,7 @@ class Archive raise UsageError, "HOMEBREW_INTERNET_ARCHIVE_KEY is unset." if key.blank? if key.exclude?(":") - raise UsageError, "Use HOMEBREW_INTERNET_ARCHIVE_KEY=access:secret. See https://archive.org/account/s3.php" + raise UsageError, "Use HOMEBREW_INTERNET_ARCHIVE_KEY=access:secret. See #{URL_PREFIX}/account/s3.php" end args += ["--header", "Authorization: AWS #{key}"] @@ -61,7 +64,7 @@ class Archive end md5_base64 = Digest::MD5.base64digest(local_file.read) - url = "https://#{@archive_item}.s3.us.archive.org/#{directory}/#{remote_file}" + url = "https://#{@archive_item}.#{S3_DOMAIN}/#{directory}/#{remote_file}" args = ["--upload-file", local_file, "--header", "Content-MD5: #{md5_base64}"] args << "--fail" unless warn_on_error result = T.unsafe(self).open_api(url, *args) @@ -82,7 +85,7 @@ class Archive formula.downloader.fetch filename = ERB::Util.url_encode(formula.downloader.basename) - destination_url = "https://archive.org/download/#{@archive_item}/#{directory}/#{filename}" + destination_url = "#{URL_PREFIX}/download/#{@archive_item}/#{directory}/#{filename}" odebug "Uploading to #{destination_url}" @@ -101,7 +104,7 @@ class Archive # @return the hash, the empty string (if the file doesn't have a hash), nil (if the file doesn't exist) sig { params(directory: String, remote_file: String).returns(T.nilable(String)) } def remote_md5(directory:, remote_file:) - url = "https://#{@archive_item}.s3.us.archive.org/#{directory}/#{remote_file}" + url = "https://#{@archive_item}.#{S3_DOMAIN}/#{directory}/#{remote_file}" result = curl_output "--fail", "--silent", "--head", "--location", url if result.success? result.stdout.match(/^ETag: "(\h{32})"/)&.values_at(1)&.first || "" @@ -116,7 +119,7 @@ class Archive def file_delete_instructions(directory, filename) <<~EOS Run: - curl -X DELETE -H "Authorization: AWS $HOMEBREW_INTERNET_ARCHIVE_KEY" https://#{@archive_item}.s3.us.archive.org/#{directory}/#{filename} + curl -X DELETE -H "Authorization: AWS $HOMEBREW_INTERNET_ARCHIVE_KEY" https://#{@archive_item}.#{S3_DOMAIN}/#{directory}/#{filename} Or run: ia delete #{@archive_item} #{directory}/#{filename} EOS diff --git a/Library/Homebrew/bintray.rb b/Library/Homebrew/bintray.rb index ddba6adcac..0290903641 100644 --- a/Library/Homebrew/bintray.rb +++ b/Library/Homebrew/bintray.rb @@ -14,6 +14,7 @@ class Bintray include Utils::Curl API_URL = "https://api.bintray.com" + URL_REGEX = %r{^https://[\w-]+\.bintray\.com/}.freeze class Error < RuntimeError end diff --git a/Library/Homebrew/dev-cmd/pr-upload.rb b/Library/Homebrew/dev-cmd/pr-upload.rb index 6e9ac110ef..03c2512ca4 100644 --- a/Library/Homebrew/dev-cmd/pr-upload.rb +++ b/Library/Homebrew/dev-cmd/pr-upload.rb @@ -4,6 +4,7 @@ require "cli/parser" require "archive" require "bintray" +require "github_releases" module Homebrew extend T::Sig @@ -14,7 +15,7 @@ module Homebrew def pr_upload_args Homebrew::CLI::Parser.new do description <<~EOS - Apply the bottle commit and publish bottles to Bintray or GitHub Releases. + Apply the bottle commit and publish bottles to a host. EOS switch "--no-publish", description: "Apply the bottle commit and upload the bottles, but don't publish them." @@ -50,22 +51,22 @@ module Homebrew end end - def archive?(bottles_hash) - @archive ||= bottles_hash.values.all? do |bottle_hash| - bottle_hash["bottle"]["root_url"].start_with? "https://archive.org/" + def internet_archive?(bottles_hash) + @internet_archive ||= bottles_hash.values.all? do |bottle_hash| + bottle_hash["bottle"]["root_url"].start_with? "#{Archive::URL_PREFIX}/" end end def bintray?(bottles_hash) @bintray ||= bottles_hash.values.all? do |bottle_hash| - bottle_hash["bottle"]["root_url"].match? %r{^https://[\w-]+\.bintray\.com/} + bottle_hash["bottle"]["root_url"].match? Bintray::URL_REGEX end end def github_releases?(bottles_hash) @github_releases ||= bottles_hash.values.all? do |bottle_hash| root_url = bottle_hash["bottle"]["root_url"] - url_match = root_url.match HOMEBREW_RELEASES_URL_REGEX + url_match = root_url.match GitHubReleases::URL_REGEX _, _, _, tag = *url_match tag @@ -92,7 +93,7 @@ module Homebrew if args.dry_run? service = - if archive?(bottles_hash) + if internet_archive?(bottles_hash) "Internet Archive" elsif bintray?(bottles_hash) "Bintray" @@ -122,44 +123,20 @@ module Homebrew safe_system HOMEBREW_BREW_FILE, *audit_args end - if archive?(bottles_hash) - # Handle uploading to the Internet Archive. + if internet_archive?(bottles_hash) archive_item = args.archive_item || "homebrew" archive = Archive.new(item: archive_item) archive.upload_bottles(bottles_hash, warn_on_error: args.warn_on_upload_failure?) elsif bintray?(bottles_hash) - # Handle uploading to Bintray. bintray_org = args.bintray_org || "homebrew" bintray = Bintray.new(org: bintray_org) bintray.upload_bottles(bottles_hash, publish_package: !args.no_publish?, warn_on_error: args.warn_on_upload_failure?) elsif github_releases?(bottles_hash) - # Handle uploading to GitHub Releases. - bottles_hash.each_value do |bottle_hash| - root_url = bottle_hash["bottle"]["root_url"] - url_match = root_url.match HOMEBREW_RELEASES_URL_REGEX - _, user, repo, tag = *url_match - - # Ensure a release is created. - release = begin - rel = GitHub.get_release user, repo, tag - odebug "Existing GitHub release \"#{tag}\" found" - rel - rescue GitHub::API::HTTPNotFoundError - odebug "Creating new GitHub release \"#{tag}\"" - GitHub.create_or_update_release user, repo, tag - end - - # Upload bottles as release assets. - bottle_hash["bottle"]["tags"].each_value do |tag_hash| - remote_file = tag_hash["filename"] - local_file = tag_hash["local_filename"] - odebug "Uploading #{remote_file}" - GitHub.upload_release_asset user, repo, release["id"], local_file: local_file, remote_file: remote_file - end - end + github_releases = GitHubReleases.new + github_releases.upload_bottles(bottles_hash) else odie "Service specified by root_url is not recognized" end diff --git a/Library/Homebrew/github_releases.rb b/Library/Homebrew/github_releases.rb new file mode 100644 index 0000000000..ae97fba4cb --- /dev/null +++ b/Library/Homebrew/github_releases.rb @@ -0,0 +1,44 @@ +# typed: false +# frozen_string_literal: true + +require "utils/github" +require "json" + +# GitHub Releases client. +# +# @api private +class GitHubReleases + extend T::Sig + + include Context + include Utils::Curl + + URL_REGEX = %r{https://github\.com/([\w-]+)/([\w-]+)?/releases/download/(.+)}.freeze + + sig { params(bottles_hash: T::Hash[String, T.untyped]).void } + def upload_bottles(bottles_hash) + bottles_hash.each_value do |bottle_hash| + root_url = bottle_hash["bottle"]["root_url"] + url_match = root_url.match URL_REGEX + _, user, repo, tag = *url_match + + # Ensure a release is created. + release = begin + rel = GitHub.get_release user, repo, tag + odebug "Existing GitHub release \"#{tag}\" found" + rel + rescue GitHub::API::HTTPNotFoundError + odebug "Creating new GitHub release \"#{tag}\"" + GitHub.create_or_update_release user, repo, tag + end + + # Upload bottles as release assets. + bottle_hash["bottle"]["tags"].each_value do |tag_hash| + remote_file = tag_hash["filename"] + local_file = tag_hash["local_filename"] + odebug "Uploading #{remote_file}" + GitHub.upload_release_asset user, repo, release["id"], local_file: local_file, remote_file: remote_file + end + end + end +end diff --git a/Library/Homebrew/global.rb b/Library/Homebrew/global.rb index 542c478c86..567b0d6f13 100644 --- a/Library/Homebrew/global.rb +++ b/Library/Homebrew/global.rb @@ -71,8 +71,6 @@ HOMEBREW_PULL_API_REGEX = %r{https://api\.github\.com/repos/([\w-]+)/([\w-]+)?/pulls/(\d+)}.freeze HOMEBREW_PULL_OR_COMMIT_URL_REGEX = %r[https://github\.com/([\w-]+)/([\w-]+)?/(?:pull/(\d+)|commit/[0-9a-fA-F]{4,40})].freeze -HOMEBREW_RELEASES_URL_REGEX = - %r{https://github\.com/([\w-]+)/([\w-]+)?/releases/download/(.+)}.freeze require "fileutils" diff --git a/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi b/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi index ce53775536..1c1b6c1af9 100644 --- a/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi +++ b/Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi @@ -12613,7 +12613,6 @@ class Object HOMEBREW_PRODUCT = ::T.let(nil, ::T.untyped) HOMEBREW_PULL_API_REGEX = ::T.let(nil, ::T.untyped) HOMEBREW_PULL_OR_COMMIT_URL_REGEX = ::T.let(nil, ::T.untyped) - HOMEBREW_RELEASES_URL_REGEX = ::T.let(nil, ::T.untyped) HOMEBREW_REPOSITORY = ::T.let(nil, ::T.untyped) HOMEBREW_REQUIRED_RUBY_VERSION = ::T.let(nil, ::T.untyped) HOMEBREW_RUBY_EXEC_ARGS = ::T.let(nil, ::T.untyped) diff --git a/completions/fish/brew.fish b/completions/fish/brew.fish index cc0f5e7fae..827d0e60e6 100644 --- a/completions/fish/brew.fish +++ b/completions/fish/brew.fish @@ -1064,7 +1064,7 @@ __fish_brew_complete_arg 'pr-pull' -l warn-on-upload-failure -d 'Warn instead of __fish_brew_complete_arg 'pr-pull' -l workflows -d 'Retrieve artifacts from the specified workflow (default: `tests.yml`). Can be a comma-separated list to include multiple workflows' -__fish_brew_complete_cmd 'pr-upload' 'Apply the bottle commit and publish bottles to Bintray or GitHub Releases' +__fish_brew_complete_cmd 'pr-upload' 'Apply the bottle commit and publish bottles to a host' __fish_brew_complete_arg 'pr-upload' -l archive-item -d 'Upload to the specified Internet Archive item (default: `homebrew`)' __fish_brew_complete_arg 'pr-upload' -l bintray-org -d 'Upload to the specified Bintray organisation (default: `homebrew`)' __fish_brew_complete_arg 'pr-upload' -l debug -d 'Display any debugging information' diff --git a/completions/zsh/_brew b/completions/zsh/_brew index 5ee2b0529b..bffa4d86a6 100644 --- a/completions/zsh/_brew +++ b/completions/zsh/_brew @@ -185,7 +185,7 @@ __brew_internal_commands() { 'pr-automerge:Find pull requests that can be automatically merged using `brew pr-publish`' 'pr-publish:Publish bottles for a pull request with GitHub Actions' 'pr-pull:Download and publish bottles, and apply the bottle commit from a pull request with artifacts generated by GitHub Actions' - 'pr-upload:Apply the bottle commit and publish bottles to Bintray or GitHub Releases' + 'pr-upload:Apply the bottle commit and publish bottles to a host' 'prof:Run Homebrew with a Ruby profiler' 'readall:Import all items from the specified tap, or from all installed taps if none is provided' 'reinstall:Uninstall and then reinstall a formula or cask using the same options it was originally installed with, plus any appended options specific to a formula' diff --git a/docs/Manpage.md b/docs/Manpage.md index c1e7136368..507b314185 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -1198,7 +1198,7 @@ Requires write access to the repository. ### `pr-upload` [*`options`*] -Apply the bottle commit and publish bottles to Bintray or GitHub Releases. +Apply the bottle commit and publish bottles to a host. * `--no-publish`: Apply the bottle commit and upload the bottles, but don't publish them. diff --git a/manpages/brew.1 b/manpages/brew.1 index e108b4ac27..d218cd8c79 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -1677,7 +1677,7 @@ Retrieve artifacts from the specified workflow (default: \fBtests\.yml\fR)\. Can Comma\-separated list of workflows which can be ignored if they have not been run\. . .SS "\fBpr\-upload\fR [\fIoptions\fR]" -Apply the bottle commit and publish bottles to Bintray or GitHub Releases\. +Apply the bottle commit and publish bottles to a host\. . .TP \fB\-\-no\-publish\fR