From 05337cbb797bab9db3234bd41b01a08477c25df1 Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Date: Wed, 17 May 2023 01:23:06 +0800
Subject: [PATCH 1/7] Refactor GitHub artifact downloads out of
 `dev-cmd/pr-pull`

I plan to use these in `test-bot` to download built bottles from
previous CI runs.
---
 Library/Homebrew/dev-cmd/pr-pull.rb   | 56 +--------------------------
 Library/Homebrew/download_strategy.rb | 34 ++++++++++++++++
 Library/Homebrew/utils/github.rb      | 23 +++++++++++
 3 files changed, 58 insertions(+), 55 deletions(-)

diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb
index 6a106d9c56..69d65eb711 100644
--- a/Library/Homebrew/dev-cmd/pr-pull.rb
+++ b/Library/Homebrew/dev-cmd/pr-pull.rb
@@ -1,7 +1,6 @@
 # typed: true
 # frozen_string_literal: true
 
-require "download_strategy"
 require "cli/parser"
 require "utils/github"
 require "tmpdir"
@@ -356,28 +355,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 +482,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, dir)
           end
 
           next if args.no_upload?
@@ -526,34 +503,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
diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index 8a0edae687..b64f9babf2 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -633,6 +633,40 @@ class CurlGitHubPackagesDownloadStrategy < CurlDownloadStrategy
   end
 end
 
+# Strategy for downloading an artifact from GitHub Actions.
+#
+# @api private
+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
+
 # Strategy for downloading a file from an Apache Mirror URL.
 #
 # @api public
diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb
index 830c6c5a81..21145f00fd 100644
--- a/Library/Homebrew/utils/github.rb
+++ b/Library/Homebrew/utils/github.rb
@@ -5,6 +5,7 @@ require "uri"
 require "utils/github/actions"
 require "utils/github/api"
 
+require "download_strategy"
 require "system_command"
 
 # Wrapper functions for the GitHub API.
@@ -371,6 +372,28 @@ module GitHub
     artifact.last["archive_download_url"]
   end
 
+  def self.download_artifact(url, artifact_id, dir = Pathname.pwd)
+    odie "Credentials must be set to access the Artifacts API" if API.credentials_type == :none
+
+    token = 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",
+        artifact_id,
+        curl_args: curl_args,
+        secrets:   [token],
+      )
+      downloader.fetch
+      downloader.stage
+    end
+  end
+
   def self.public_member_usernames(org, per_page: 100)
     url = "#{API_URL}/orgs/#{org}/public_members"
     members = []

From d372eb86ca176c1f6271ac802d0d843e98f9e8be Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Date: Wed, 17 May 2023 11:39:40 +0800
Subject: [PATCH 2/7] utils/github: remove unnecessary `chdir` call

The only existing caller already has the requested directory as the
workign directory, so this is currently not needed.
---
 Library/Homebrew/dev-cmd/pr-pull.rb |  2 +-
 Library/Homebrew/utils/github.rb    | 29 +++++++++++++++++------------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb
index 69d65eb711..99991c34c8 100644
--- a/Library/Homebrew/dev-cmd/pr-pull.rb
+++ b/Library/Homebrew/dev-cmd/pr-pull.rb
@@ -482,7 +482,7 @@ module Homebrew
 
             ohai "Downloading bottles for workflow: #{workflow}"
             url = GitHub.get_artifact_url(workflow_run)
-            GitHub.download_artifact(url, pr, dir)
+            GitHub.download_artifact(url, pr)
           end
 
           next if args.no_upload?
diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb
index 21145f00fd..eb3ec64543 100644
--- a/Library/Homebrew/utils/github.rb
+++ b/Library/Homebrew/utils/github.rb
@@ -372,7 +372,14 @@ module GitHub
     artifact.last["archive_download_url"]
   end
 
-  def self.download_artifact(url, artifact_id, dir = Pathname.pwd)
+  # Downloads an artifact from GitHub Actions.
+  #
+  # @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)
     odie "Credentials must be set to access the Artifacts API" if API.credentials_type == :none
 
     token = API.credentials
@@ -381,17 +388,15 @@ module GitHub
     # 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",
-        artifact_id,
-        curl_args: curl_args,
-        secrets:   [token],
-      )
-      downloader.fetch
-      downloader.stage
-    end
+    downloader = GitHubArtifactDownloadStrategy.new(
+      url,
+      "artifact",
+      artifact_id,
+      curl_args: curl_args,
+      secrets:   [token],
+    )
+    downloader.fetch
+    downloader.stage
   end
 
   def self.public_member_usernames(org, per_page: 100)

From ee6ef00a73662606e9902a37efbf3e3f89dd5620 Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Date: Wed, 17 May 2023 11:43:14 +0800
Subject: [PATCH 3/7] GitHubArtifactDownloadStrategy: set cache properly

Co-authored-by: Bo Anderson <mail@boanderson.me>
---
 Library/Homebrew/download_strategy.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index b64f9babf2..884c169ed2 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -637,6 +637,11 @@ end
 #
 # @api private
 class GitHubArtifactDownloadStrategy < AbstractFileDownloadStrategy
+  def initialize
+    super
+    @cache = HOMEBREW_CACHE/"gh-actions-artifact"
+  end
+
   def fetch(timeout: nil)
     ohai "Downloading #{url}"
     if cached_location.exist?

From 9260bd97e89a198b9a609046afc0355f20e36bf0 Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Date: Wed, 17 May 2023 12:58:31 +0800
Subject: [PATCH 4/7] GitHubArtifactDownloadStrategy: fix `initialize`
 signature

---
 Library/Homebrew/download_strategy.rb | 9 +++++----
 Library/Homebrew/utils/github.rb      | 9 +--------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index 884c169ed2..b28410150e 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -637,9 +637,10 @@ end
 #
 # @api private
 class GitHubArtifactDownloadStrategy < AbstractFileDownloadStrategy
-  def initialize
-    super
+  def initialize(url, artifact_id, token:)
+    super(url, "artifact", artifact_id)
     @cache = HOMEBREW_CACHE/"gh-actions-artifact"
+    @token = token
   end
 
   def fetch(timeout: nil)
@@ -649,8 +650,8 @@ class GitHubArtifactDownloadStrategy < AbstractFileDownloadStrategy
     else
       begin
         curl "--location", "--create-dirs", "--output", temporary_path, url,
-             *meta.fetch(:curl_args, []),
-             secrets: meta.fetch(:secrets, []),
+             "--header", "Authorization: token #{@token}",
+             secrets: [@token],
              timeout: timeout
       rescue ErrorDuringExecution
         raise CurlDownloadStrategyError, url
diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb
index eb3ec64543..9ab210b630 100644
--- a/Library/Homebrew/utils/github.rb
+++ b/Library/Homebrew/utils/github.rb
@@ -383,18 +383,11 @@ module GitHub
     odie "Credentials must be set to access the Artifacts API" if API.credentials_type == :none
 
     token = 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.
-    downloader = GitHubArtifactDownloadStrategy.new(
-      url,
-      "artifact",
-      artifact_id,
-      curl_args: curl_args,
-      secrets:   [token],
-    )
+    downloader = GitHubArtifactDownloadStrategy.new(url, artifact_id, token: token)
     downloader.fetch
     downloader.stage
   end

From 43b6d79a4cf575439d3d3a7858c3e034ab274ebb Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Date: Wed, 17 May 2023 13:27:10 +0800
Subject: [PATCH 5/7] cleanup: handle GitHub Actions artifacts

Implemented based on feedback from #15440.
---
 Library/Homebrew/cleanup.rb | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Library/Homebrew/cleanup.rb b/Library/Homebrew/cleanup.rb
index f0f16d08b9..8c39af2c64 100644
--- a/Library/Homebrew/cleanup.rb
+++ b/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")

From d7870bb24d43ffac248965c9d0a100a75f4f431b Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Date: Wed, 17 May 2023 23:48:58 +0800
Subject: [PATCH 6/7] Move artifact download code to separate file

This will help avoid mutually-recursive `require`s.
---
 Library/Homebrew/dev-cmd/pr-pull.rb        |  1 +
 Library/Homebrew/download_strategy.rb      | 40 -------------
 Library/Homebrew/utils/github.rb           | 21 -------
 Library/Homebrew/utils/github/artifacts.rb | 67 ++++++++++++++++++++++
 4 files changed, 68 insertions(+), 61 deletions(-)
 create mode 100644 Library/Homebrew/utils/github/artifacts.rb

diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb
index 99991c34c8..4aaee271e5 100644
--- a/Library/Homebrew/dev-cmd/pr-pull.rb
+++ b/Library/Homebrew/dev-cmd/pr-pull.rb
@@ -3,6 +3,7 @@
 
 require "cli/parser"
 require "utils/github"
+require "utils/github/artifacts"
 require "tmpdir"
 require "formula"
 
diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index b28410150e..8a0edae687 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -633,46 +633,6 @@ class CurlGitHubPackagesDownloadStrategy < CurlDownloadStrategy
   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
-
 # Strategy for downloading a file from an Apache Mirror URL.
 #
 # @api public
diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb
index 9ab210b630..830c6c5a81 100644
--- a/Library/Homebrew/utils/github.rb
+++ b/Library/Homebrew/utils/github.rb
@@ -5,7 +5,6 @@ require "uri"
 require "utils/github/actions"
 require "utils/github/api"
 
-require "download_strategy"
 require "system_command"
 
 # Wrapper functions for the GitHub API.
@@ -372,26 +371,6 @@ module GitHub
     artifact.last["archive_download_url"]
   end
 
-  # Downloads an artifact from GitHub Actions.
-  #
-  # @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)
-    odie "Credentials must be set to access the Artifacts API" if API.credentials_type == :none
-
-    token = API.credentials
-
-    # 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.
-    downloader = GitHubArtifactDownloadStrategy.new(url, artifact_id, token: token)
-    downloader.fetch
-    downloader.stage
-  end
-
   def self.public_member_usernames(org, per_page: 100)
     url = "#{API_URL}/orgs/#{org}/public_members"
     members = []
diff --git a/Library/Homebrew/utils/github/artifacts.rb b/Library/Homebrew/utils/github/artifacts.rb
new file mode 100644
index 0000000000..7cae2213a3
--- /dev/null
+++ b/Library/Homebrew/utils/github/artifacts.rb
@@ -0,0 +1,67 @@
+# typed: true
+# frozen_string_literal: true
+
+require "download_strategy"
+require "utils/github"
+
+module GitHub
+  # Downloads an artifact from GitHub Actions.
+  #
+  # @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)
+    odie "Credentials must be set to access the Artifacts API" if API.credentials_type == :none
+
+    token = API.credentials
+
+    # 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.
+    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

From 4c0912d95cb2f8002b4760d28a7587198c28aff5 Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Date: Wed, 17 May 2023 23:54:45 +0800
Subject: [PATCH 7/7] utils/github/artifacts: minor improvements

---
 Library/Homebrew/utils/github/artifacts.rb | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Library/Homebrew/utils/github/artifacts.rb b/Library/Homebrew/utils/github/artifacts.rb
index 7cae2213a3..f6cf42365c 100644
--- a/Library/Homebrew/utils/github/artifacts.rb
+++ b/Library/Homebrew/utils/github/artifacts.rb
@@ -5,7 +5,7 @@ require "download_strategy"
 require "utils/github"
 
 module GitHub
-  # Downloads an artifact from GitHub Actions.
+  # 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
@@ -13,13 +13,11 @@ module GitHub
   # @api private
   sig { params(url: String, artifact_id: String).void }
   def self.download_artifact(url, artifact_id)
-    odie "Credentials must be set to access the Artifacts API" if API.credentials_type == :none
+    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
-
-    # 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.
     downloader = GitHubArtifactDownloadStrategy.new(url, artifact_id, token: token)
     downloader.fetch
     downloader.stage