Merge pull request #15440 from carlocab/download-artifact-refactor

Refactor GitHub artifact downloads out of `dev-cmd/pr-pull`

Library/Homebrew/cleanup.rb

@@ -61,6 +61,8 @@ module Homebrew
           stale_api_source?(pathname, scrub)
         when :cask
           stale_cask?(pathname, scrub)
+        when :gh_actions_artifact
+          stale_gh_actions_artifact?(pathname, scrub)
         else
           stale_formula?(pathname, scrub)
         end
@@ -68,6 +70,13 @@ module Homebrew
 
       private
 
+      GH_ACTIONS_ARTIFACT_CLEANUP_DAYS = 3
+
+      sig { params(pathname: Pathname, scrub: T::Boolean).returns(T::Boolean) }
+      def stale_gh_actions_artifact?(pathname, scrub)
+        scrub || prune?(pathname, GH_ACTIONS_ARTIFACT_CLEANUP_DAYS)
+      end
+
       sig { params(pathname: Pathname, scrub: T::Boolean).returns(T::Boolean) }
       def stale_api_source?(pathname, scrub)
         return true if scrub
@@ -350,10 +359,12 @@ module Homebrew
       files = cache.directory? ? cache.children : []
       cask_files = (cache/"Cask").directory? ? (cache/"Cask").children : []
       api_source_files = (cache/"api-source").glob("*/*/*/*/*") # org/repo/git_head/type/file.rb
+      gh_actions_artifacts = (cache/"gh-actions-artifact").directory? ? (cache/"gh-actions-artifact").children : []
 
       files.map { |path| { path: path, type: nil } } +
         cask_files.map { |path| { path: path, type: :cask } } +
-        api_source_files.map { |path| { path: path, type: :api_source } }
+        api_source_files.map { |path| { path: path, type: :api_source } } +
+        gh_actions_artifacts.map { |path| { path: path, type: :gh_actions_artifact } }
     end
 
     def cleanup_empty_api_source_directories(directory = cache/"api-source")

Library/Homebrew/dev-cmd/pr-pull.rb

@@ -1,9 +1,9 @@
 # typed: true
 # frozen_string_literal: true
 
-require "download_strategy"
 require "cli/parser"
 require "utils/github"
+require "utils/github/artifacts"
 require "tmpdir"
 require "formula"
 
@@ -356,28 +356,6 @@ module Homebrew
     formulae + casks
   end
 
-  def self.download_artifact(url, dir, pull_request)
-    odie "Credentials must be set to access the Artifacts API" if GitHub::API.credentials_type == :none
-
-    token = GitHub::API.credentials
-    curl_args = ["--header", "Authorization: token #{token}"]
-
-    # 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",
-        pull_request,
-        curl_args: curl_args,
-        secrets:   [token],
-      )
-      downloader.fetch
-      downloader.stage
-    end
-  end
-
   def self.pr_check_conflicts(repo, pull_request)
     long_build_pr_files = GitHub.issues(
       repo: repo, state: "open", labels: "no long build conflict",
@@ -505,7 +483,7 @@ module Homebrew
 
             ohai "Downloading bottles for workflow: #{workflow}"
             url = GitHub.get_artifact_url(workflow_run)
-            download_artifact(url, dir, pr)
+            GitHub.download_artifact(url, pr)
           end
 
           next if args.no_upload?
@@ -526,34 +504,3 @@ module Homebrew
     end
   end
 end
-
-class GitHubArtifactDownloadStrategy < AbstractFileDownloadStrategy
-  def fetch(timeout: nil)
-    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, []),
-             timeout: timeout
-      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
-
-  sig { returns(String) }
-  def resolved_basename
-    "artifact.zip"
-  end
-end

Library/Homebrew/utils/github/artifacts.rb

@@ -0,0 +1,65 @@
+# typed: true
+# frozen_string_literal: true
+
+require "download_strategy"
+require "utils/github"
+
+module GitHub
+  # Download an artifact from GitHub Actions and unpack it into the current working directory.
+  #
+  # @param url [String] URL to download from
+  # @param artifact_id [String] a value that uniquely identifies the downloaded artifact
+  #
+  # @api private
+  sig { params(url: String, artifact_id: String).void }
+  def self.download_artifact(url, artifact_id)
+    raise API::MissingAuthenticationError if API.credentials == :none
+
+    # We use a download strategy here to leverage the Homebrew cache
+    # to avoid repeated downloads of (possibly large) bottles.
+    token = API.credentials
+    downloader = GitHubArtifactDownloadStrategy.new(url, artifact_id, token: token)
+    downloader.fetch
+    downloader.stage
+  end
+end
+
+# Strategy for downloading an artifact from GitHub Actions.
+#
+# @api private
+class GitHubArtifactDownloadStrategy < AbstractFileDownloadStrategy
+  def initialize(url, artifact_id, token:)
+    super(url, "artifact", artifact_id)
+    @cache = HOMEBREW_CACHE/"gh-actions-artifact"
+    @token = token
+  end
+
+  def fetch(timeout: nil)
+    ohai "Downloading #{url}"
+    if cached_location.exist?
+      puts "Already downloaded: #{cached_location}"
+    else
+      begin
+        curl "--location", "--create-dirs", "--output", temporary_path, url,
+             "--header", "Authorization: token #{@token}",
+             secrets: [@token],
+             timeout: timeout
+      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
+
+  sig { returns(String) }
+  def resolved_basename
+    "artifact.zip"
+  end
+end