Merge pull request #10825 from MikeMcQuaid/pr-upload-cleanup

dev-cmd/pr-upload: some refactoring

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
     "#<Archive: item=#{@archive_item}>"
@@ -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

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

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

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

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"
 

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)

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'

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'

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.

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