From 0df71ea6a32f840873406adf200bcc1de43f67ca Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Wed, 17 Apr 2024 02:57:56 +0800 Subject: [PATCH 1/7] utils/github: support globbing artifacts --- Library/Homebrew/test/utils/github_spec.rb | 26 ++++++++++++++-------- Library/Homebrew/utils/github.rb | 18 +++++++-------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/Library/Homebrew/test/utils/github_spec.rb b/Library/Homebrew/test/utils/github_spec.rb index 1c01709361..56a047c5af 100644 --- a/Library/Homebrew/test/utils/github_spec.rb +++ b/Library/Homebrew/test/utils/github_spec.rb @@ -46,10 +46,10 @@ RSpec.describe GitHub do end end - describe "::get_artifact_url", :needs_network do + describe "::get_artifact_urls", :needs_network do it "fails to find a nonexistent workflow" do expect do - described_class.get_artifact_url( + described_class.get_artifact_urls( described_class.get_workflow_run("Homebrew", "homebrew-core", "1"), ) end.to raise_error(/No matching check suite found/) @@ -57,19 +57,27 @@ RSpec.describe GitHub do it "fails to find artifacts that don't exist" do expect do - described_class.get_artifact_url( + described_class.get_artifact_urls( described_class.get_workflow_run("Homebrew", "homebrew-core", "135608", - workflow_id: "triage.yml", artifact_name: "false_artifact"), + workflow_id: "triage.yml", artifact_pattern: "false_artifact"), ) - end.to raise_error(/No artifact .+ was found/) + end.to raise_error(/No artifacts with the pattern .+ were found/) end - it "gets an artifact link" do - url = described_class.get_artifact_url( + it "gets artifact URLs" do + urls = described_class.get_artifact_urls( described_class.get_workflow_run("Homebrew", "homebrew-core", "135608", - workflow_id: "triage.yml", artifact_name: "event_payload"), + workflow_id: "triage.yml", artifact_pattern: "event_payload"), ) - expect(url).to eq("https://api.github.com/repos/Homebrew/homebrew-core/actions/artifacts/781984175/zip") + expect(urls).to eq(["https://api.github.com/repos/Homebrew/homebrew-core/actions/artifacts/781984175/zip"]) + end + + it "supports pattern matching" do + urls = described_class.get_artifact_urls( + described_class.get_workflow_run("Homebrew", "brew", "17068", + workflow_id: "pkg-installer.yml", artifact_pattern: "Homebrew-*.pkg"), + ) + expect(urls).to eq(["https://api.github.com/repos/Homebrew/brew/actions/artifacts/1405050842/zip"]) end end diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index 17f6ec40b0..de01f007c6 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -283,7 +283,7 @@ module GitHub API.open_rest(url, data_binary_path: local_file, request_method: :POST, scopes: CREATE_ISSUE_FORK_OR_PR_SCOPES) end - def self.get_workflow_run(user, repo, pull_request, workflow_id: "tests.yml", artifact_name: "bottles") + def self.get_workflow_run(user, repo, pull_request, workflow_id: "tests.yml", artifact_pattern: "bottles{,-*}") scopes = CREATE_ISSUE_FORK_OR_PR_SCOPES # GraphQL unfortunately has no way to get the workflow yml name, so we need an extra REST call. @@ -333,11 +333,11 @@ module GitHub [] end - [check_suite, user, repo, pull_request, workflow_id, scopes, artifact_name] + [check_suite, user, repo, pull_request, workflow_id, scopes, artifact_pattern] end - def self.get_artifact_url(workflow_array) - check_suite, user, repo, pr, workflow_id, scopes, artifact_name = *workflow_array + def self.get_artifact_urls(workflow_array) + check_suite, user, repo, pr, workflow_id, scopes, artifact_pattern = *workflow_array if check_suite.empty? raise API::Error, <<~EOS No matching check suite found for these criteria! @@ -357,18 +357,18 @@ module GitHub run_id = check_suite.last["workflowRun"]["databaseId"] artifacts = API.open_rest("#{API_URL}/repos/#{user}/#{repo}/actions/runs/#{run_id}/artifacts", scopes:) - artifact = artifacts["artifacts"].select do |art| - art["name"] == artifact_name + matching_artifacts = artifacts["artifacts"].select do |art| + File.fnmatch?(artifact_pattern, art["name"], File::FNM_EXTGLOB) end - if artifact.empty? + if matching_artifacts.empty? raise API::Error, <<~EOS - No artifact with the name `#{artifact_name}` was found! + No artifacts with the pattern `#{artifact_pattern}` were found! #{Formatter.url check_suite.last["workflowRun"]["url"]} EOS end - artifact.last["archive_download_url"] + matching_artifacts.map { |art| art["archive_download_url"] } end def self.public_member_usernames(org, per_page: 100) From 1a1a466e9d23c921fe4c7ddf08a9bf777472f57c Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Wed, 17 Apr 2024 03:32:48 +0800 Subject: [PATCH 2/7] pr-pull: support globbing artifacts Artifact actions v3 is deprecated and will soon be unavailable [^1]. This adds support for v4 by allowing `brew pr-pull` to accept a glob pattern for artifact names, like actions/download-artifact does [^2]. [^1]: https://github.blog/changelog/2024-04-16-deprecation-notice-v3-of-the-artifact-actions/ [^2]: https://github.com/actions/download-artifact/tree/v4/#usage --- Library/Homebrew/dev-cmd/pr-pull.rb | 14 ++++++++------ .../sorbet/rbi/dsl/homebrew/dev_cmd/pr_pull.rbi | 3 +++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index da8444543d..7bed63570f 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -55,8 +55,8 @@ module Homebrew flag "--message=", depends_on: "--autosquash", description: "Message to include when autosquashing revision bumps, deletions and rebuilds." - flag "--artifact=", - description: "Download artifacts with the specified name (default: `bottles`)." + flag "--artifact-pattern=", "--artifact=", + description: "Download artifacts with the specified pattern (default: `bottles{,-*}`)." flag "--tap=", description: "Target tap repository (default: `homebrew/core`)." flag "--root-url=", @@ -81,7 +81,7 @@ module Homebrew ensure_executable!("unzip", reason: "extracting CI artifacts") workflows = args.workflows.presence || ["tests.yml"] - artifact = args.artifact || "bottles" + artifact_pattern = args.artifact_pattern || "bottles{,-*}" tap = Tap.fetch(args.tap || CoreTap.instance.name) raise TapUnavailableError, tap.name unless tap.installed? @@ -143,7 +143,7 @@ module Homebrew workflows.each do |workflow| workflow_run = GitHub.get_workflow_run( - user, repo, pr, workflow_id: workflow, artifact_name: artifact + user, repo, pr, workflow_id: workflow, artifact_pattern: ) if args.ignore_missing_artifacts.present? && T.must(args.ignore_missing_artifacts).include?(workflow) && @@ -155,8 +155,10 @@ module Homebrew end ohai "Downloading bottles for workflow: #{workflow}" - url = GitHub.get_artifact_url(workflow_run) - GitHub.download_artifact(url, pr) + urls = GitHub.get_artifact_urls(workflow_run) + urls.each do |url| + GitHub.download_artifact(url, pr) + end end next if args.no_upload? diff --git a/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/pr_pull.rbi b/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/pr_pull.rbi index c1ccfee16c..fc8f719367 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/pr_pull.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/pr_pull.rbi @@ -8,6 +8,9 @@ class Homebrew::CLI::Args sig { returns(T.nilable(String)) } def artifact; end + sig { returns(T.nilable(String)) } + def artifact_pattern; end + sig { returns(T::Boolean) } def autosquash?; end From d6d1b7d4e9485cdb8f79cb2cfac11843e4f854bb Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Wed, 17 Apr 2024 03:33:05 +0800 Subject: [PATCH 3/7] Update manpage and completions --- completions/bash/brew | 2 +- completions/fish/brew.fish | 2 +- completions/zsh/_brew | 2 +- docs/Manpage.md | 4 ++-- manpages/brew.1 | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/completions/bash/brew b/completions/bash/brew index b539b283fa..318ae832a4 100644 --- a/completions/bash/brew +++ b/completions/bash/brew @@ -1760,7 +1760,7 @@ _brew_pr_pull() { case "${cur}" in -*) __brewcomp " - --artifact + --artifact-pattern --autosquash --branch-okay --clean diff --git a/completions/fish/brew.fish b/completions/fish/brew.fish index 52c3fc97e2..bcb88f61a1 100644 --- a/completions/fish/brew.fish +++ b/completions/fish/brew.fish @@ -1191,7 +1191,7 @@ __fish_brew_complete_arg 'pr-publish' -l workflow -d 'Target workflow filename ( __fish_brew_complete_cmd 'pr-pull' 'Download and publish bottles, and apply the bottle commit from a pull request with artifacts generated by GitHub Actions' -__fish_brew_complete_arg 'pr-pull' -l artifact -d 'Download artifacts with the specified name (default: `bottles`)' +__fish_brew_complete_arg 'pr-pull' -l artifact-pattern -d 'Download artifacts with the specified pattern (default: `bottles{,-*}`)' __fish_brew_complete_arg 'pr-pull' -l autosquash -d 'Automatically reformat and reword commits in the pull request to our preferred format' __fish_brew_complete_arg 'pr-pull' -l branch-okay -d 'Do not warn if pulling to a branch besides the repository default (useful for testing)' __fish_brew_complete_arg 'pr-pull' -l clean -d 'Do not amend the commits from pull requests' diff --git a/completions/zsh/_brew b/completions/zsh/_brew index f1d3e95448..99a3d4ac16 100644 --- a/completions/zsh/_brew +++ b/completions/zsh/_brew @@ -1482,7 +1482,7 @@ _brew_pr_publish() { # brew pr-pull _brew_pr_pull() { _arguments \ - '--artifact[Download artifacts with the specified name (default: `bottles`)]' \ + '--artifact-pattern[Download artifacts with the specified pattern (default: `bottles{,-*}`)]' \ '(--clean)--autosquash[Automatically reformat and reword commits in the pull request to our preferred format]' \ '--branch-okay[Do not warn if pulling to a branch besides the repository default (useful for testing)]' \ '(--autosquash)--clean[Do not amend the commits from pull requests]' \ diff --git a/docs/Manpage.md b/docs/Manpage.md index 427057872a..bbfd28101a 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -2423,9 +2423,9 @@ repository. : Message to include when autosquashing revision bumps, deletions and rebuilds. -`--artifact` +`--artifact-pattern` -: Download artifacts with the specified name (default: `bottles`). +: Download artifacts with the specified pattern (default: `bottles{,-*}`). `--tap` diff --git a/manpages/brew.1 b/manpages/brew.1 index b12240eae6..63de44ddd5 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -1543,8 +1543,8 @@ Specify a committer name and email in \fBgit\fP\[u2019]s standard author format\ \fB\-\-message\fP Message to include when autosquashing revision bumps, deletions and rebuilds\. .TP -\fB\-\-artifact\fP -Download artifacts with the specified name (default: \fBbottles\fP)\. +\fB\-\-artifact\-pattern\fP +Download artifacts with the specified pattern (default: \fBbottles{,\-*}\fP)\. .TP \fB\-\-tap\fP Target tap repository (default: \fBhomebrew/core\fP)\. From dd92ad8e1b8ea180533fe130e6930509b6790ae7 Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Wed, 17 Apr 2024 05:52:11 +0800 Subject: [PATCH 4/7] Update default artifact pattern to avoid migration problems --- Library/Homebrew/dev-cmd/pr-pull.rb | 5 +++-- Library/Homebrew/utils/github.rb | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 7bed63570f..5f1cafa3ca 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -56,7 +56,7 @@ module Homebrew depends_on: "--autosquash", description: "Message to include when autosquashing revision bumps, deletions and rebuilds." flag "--artifact-pattern=", "--artifact=", - description: "Download artifacts with the specified pattern (default: `bottles{,-*}`)." + description: "Download artifacts with the specified pattern (default: `bottles{,_*}`)." flag "--tap=", description: "Target tap repository (default: `homebrew/core`)." flag "--root-url=", @@ -81,7 +81,7 @@ module Homebrew ensure_executable!("unzip", reason: "extracting CI artifacts") workflows = args.workflows.presence || ["tests.yml"] - artifact_pattern = args.artifact_pattern || "bottles{,-*}" + artifact_pattern = args.artifact_pattern || "bottles{,_*}" tap = Tap.fetch(args.tap || CoreTap.instance.name) raise TapUnavailableError, tap.name unless tap.installed? @@ -155,6 +155,7 @@ module Homebrew end ohai "Downloading bottles for workflow: #{workflow}" + urls = GitHub.get_artifact_urls(workflow_run) urls.each do |url| GitHub.download_artifact(url, pr) diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index de01f007c6..a090cf87b3 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -283,7 +283,7 @@ module GitHub API.open_rest(url, data_binary_path: local_file, request_method: :POST, scopes: CREATE_ISSUE_FORK_OR_PR_SCOPES) end - def self.get_workflow_run(user, repo, pull_request, workflow_id: "tests.yml", artifact_pattern: "bottles{,-*}") + def self.get_workflow_run(user, repo, pull_request, workflow_id: "tests.yml", artifact_pattern: "bottles{,_*}") scopes = CREATE_ISSUE_FORK_OR_PR_SCOPES # GraphQL unfortunately has no way to get the workflow yml name, so we need an extra REST call. From 135db8c559e9ad21216123a2f48810d3cc29903d Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Wed, 17 Apr 2024 05:59:29 +0800 Subject: [PATCH 5/7] Update manpage and completions --- completions/fish/brew.fish | 2 +- completions/zsh/_brew | 2 +- docs/Manpage.md | 2 +- manpages/brew.1 | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/completions/fish/brew.fish b/completions/fish/brew.fish index bcb88f61a1..9167474328 100644 --- a/completions/fish/brew.fish +++ b/completions/fish/brew.fish @@ -1191,7 +1191,7 @@ __fish_brew_complete_arg 'pr-publish' -l workflow -d 'Target workflow filename ( __fish_brew_complete_cmd 'pr-pull' 'Download and publish bottles, and apply the bottle commit from a pull request with artifacts generated by GitHub Actions' -__fish_brew_complete_arg 'pr-pull' -l artifact-pattern -d 'Download artifacts with the specified pattern (default: `bottles{,-*}`)' +__fish_brew_complete_arg 'pr-pull' -l artifact-pattern -d 'Download artifacts with the specified pattern (default: `bottles{,_*}`)' __fish_brew_complete_arg 'pr-pull' -l autosquash -d 'Automatically reformat and reword commits in the pull request to our preferred format' __fish_brew_complete_arg 'pr-pull' -l branch-okay -d 'Do not warn if pulling to a branch besides the repository default (useful for testing)' __fish_brew_complete_arg 'pr-pull' -l clean -d 'Do not amend the commits from pull requests' diff --git a/completions/zsh/_brew b/completions/zsh/_brew index 99a3d4ac16..65e86cf5b5 100644 --- a/completions/zsh/_brew +++ b/completions/zsh/_brew @@ -1482,7 +1482,7 @@ _brew_pr_publish() { # brew pr-pull _brew_pr_pull() { _arguments \ - '--artifact-pattern[Download artifacts with the specified pattern (default: `bottles{,-*}`)]' \ + '--artifact-pattern[Download artifacts with the specified pattern (default: `bottles{,_*}`)]' \ '(--clean)--autosquash[Automatically reformat and reword commits in the pull request to our preferred format]' \ '--branch-okay[Do not warn if pulling to a branch besides the repository default (useful for testing)]' \ '(--autosquash)--clean[Do not amend the commits from pull requests]' \ diff --git a/docs/Manpage.md b/docs/Manpage.md index bbfd28101a..529a11a10e 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -2425,7 +2425,7 @@ repository. `--artifact-pattern` -: Download artifacts with the specified pattern (default: `bottles{,-*}`). +: Download artifacts with the specified pattern (default: `bottles{,_*}`). `--tap` diff --git a/manpages/brew.1 b/manpages/brew.1 index 63de44ddd5..16e11d1742 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -1544,7 +1544,7 @@ Specify a committer name and email in \fBgit\fP\[u2019]s standard author format\ Message to include when autosquashing revision bumps, deletions and rebuilds\. .TP \fB\-\-artifact\-pattern\fP -Download artifacts with the specified pattern (default: \fBbottles{,\-*}\fP)\. +Download artifacts with the specified pattern (default: \fBbottles{,_*}\fP)\. .TP \fB\-\-tap\fP Target tap repository (default: \fBhomebrew/core\fP)\. From 9ad60fe437c0d524836147086c528cc86affc51f Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Wed, 17 Apr 2024 07:35:40 +0800 Subject: [PATCH 6/7] utils/github: avoid returning artifacts with the same name --- Library/Homebrew/utils/github.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index a090cf87b3..d2cde79847 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -357,9 +357,11 @@ module GitHub run_id = check_suite.last["workflowRun"]["databaseId"] artifacts = API.open_rest("#{API_URL}/repos/#{user}/#{repo}/actions/runs/#{run_id}/artifacts", scopes:) - matching_artifacts = artifacts["artifacts"].select do |art| - File.fnmatch?(artifact_pattern, art["name"], File::FNM_EXTGLOB) - end + matching_artifacts = + artifacts["artifacts"] + .group_by { |art| art["name"] } + .select { |name| File.fnmatch?(artifact_pattern, name, File::FNM_EXTGLOB) } + .map { |_, arts| arts.last } if matching_artifacts.empty? raise API::Error, <<~EOS From 4cf42f7c1825d7a6df7ec6cae9f9c48267ab8556 Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Wed, 17 Apr 2024 07:37:02 +0800 Subject: [PATCH 7/7] pr-pull: simplify --- Library/Homebrew/dev-cmd/pr-pull.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 5f1cafa3ca..c08badc32f 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -157,9 +157,7 @@ module Homebrew ohai "Downloading bottles for workflow: #{workflow}" urls = GitHub.get_artifact_urls(workflow_run) - urls.each do |url| - GitHub.download_artifact(url, pr) - end + urls.each { |url| GitHub.download_artifact(url, pr) } end next if args.no_upload?