From 4e61f61a208c0074bdf60fc68b7bb0e88e67c3d7 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sun, 21 Mar 2021 12:35:45 -0400 Subject: [PATCH] utils/github: handle non-standard tap remotes --- Library/Homebrew/dev-cmd/bump-cask-pr.rb | 3 +- Library/Homebrew/dev-cmd/bump-formula-pr.rb | 38 ++++++++++----------- Library/Homebrew/dev-cmd/bump.rb | 3 +- Library/Homebrew/tap.rb | 8 +++++ Library/Homebrew/test/tap_spec.rb | 27 +++++++++++++++ Library/Homebrew/utils/github.rb | 26 +++++++------- 6 files changed, 71 insertions(+), 34 deletions(-) diff --git a/Library/Homebrew/dev-cmd/bump-cask-pr.rb b/Library/Homebrew/dev-cmd/bump-cask-pr.rb index cd4ca2cb35..abdbf0e8a8 100644 --- a/Library/Homebrew/dev-cmd/bump-cask-pr.rb +++ b/Library/Homebrew/dev-cmd/bump-cask-pr.rb @@ -199,7 +199,8 @@ module Homebrew end def check_open_pull_requests(cask, args:) - GitHub.check_for_duplicate_pull_requests(cask.token, cask.tap.full_name, + tap_remote_repo = cask.tap.remote_repo || cask.tap.full_name + GitHub.check_for_duplicate_pull_requests(cask.token, tap_remote_repo, state: "open", file: cask.sourcefile_path.relative_path_from(cask.tap.path).to_s, args: args) diff --git a/Library/Homebrew/dev-cmd/bump-formula-pr.rb b/Library/Homebrew/dev-cmd/bump-formula-pr.rb index f88704091a..671dcb3133 100644 --- a/Library/Homebrew/dev-cmd/bump-formula-pr.rb +++ b/Library/Homebrew/dev-cmd/bump-formula-pr.rb @@ -85,10 +85,10 @@ module Homebrew def use_correct_linux_tap(formula, args:) default_origin_branch = formula.tap.path.git_origin_branch - return formula.tap.full_name, "origin", default_origin_branch, "-" if !OS.linux? || !formula.tap.core_tap? + return formula.tap.remote_repo, "origin", default_origin_branch, "-" if !OS.linux? || !formula.tap.core_tap? - tap_full_name = formula.tap.full_name.gsub("linuxbrew", "homebrew") - homebrew_core_url = "https://github.com/#{tap_full_name}" + tap_remote_repo = formula.tap.full_name.gsub("linuxbrew", "homebrew") + homebrew_core_url = "https://github.com/#{tap_remote_repo}" homebrew_core_remote = "homebrew" previous_branch = formula.tap.path.git_branch || "master" formula_path = formula.path.relative_path_from(formula.tap.path) @@ -99,7 +99,7 @@ module Homebrew ohai "git fetch #{homebrew_core_remote} HEAD #{default_origin_branch}" ohai "git cat-file -e #{full_origin_branch}:#{formula_path}" ohai "git checkout #{full_origin_branch}" - return tap_full_name, homebrew_core_remote, default_origin_branch, previous_branch + return tap_remote_repo, homebrew_core_remote, default_origin_branch, previous_branch end formula.tap.path.cd do @@ -112,7 +112,7 @@ module Homebrew if quiet_system "git", "cat-file", "-e", "#{full_origin_branch}:#{formula_path}" ohai "#{formula.full_name} exists in #{full_origin_branch}." safe_system "git", "checkout", full_origin_branch - return tap_full_name, homebrew_core_remote, default_origin_branch, previous_branch + return tap_remote_repo, homebrew_core_remote, default_origin_branch, previous_branch end end end @@ -145,11 +145,11 @@ module Homebrew formula_spec = formula.stable odie "#{formula}: no stable specification found!" if formula_spec.blank? - tap_full_name, remote, remote_branch, previous_branch = use_correct_linux_tap(formula, args: args) - check_open_pull_requests(formula, tap_full_name, args: args) + tap_remote_repo, remote, remote_branch, previous_branch = use_correct_linux_tap(formula, args: args) + check_open_pull_requests(formula, tap_remote_repo, args: args) new_version = args.version - check_new_version(formula, tap_full_name, version: new_version, args: args) if new_version.present? + check_new_version(formula, tap_remote_repo, version: new_version, args: args) if new_version.present? opoo "This formula has patches that may be resolved upstream." if formula.patchlist.present? if formula.resources.any? { |resource| !resource.name.start_with?("homebrew-") } @@ -173,10 +173,10 @@ module Homebrew old_version = old_formula_version.to_s forced_version = new_version.present? new_url_hash = if new_url.present? && new_hash.present? - check_new_version(formula, tap_full_name, url: new_url, args: args) if new_version.blank? + check_new_version(formula, tap_remote_repo, url: new_url, args: args) if new_version.blank? true elsif new_tag.present? && new_revision.present? - check_new_version(formula, tap_full_name, url: old_url, tag: new_tag, args: args) if new_version.blank? + check_new_version(formula, tap_remote_repo, url: old_url, tag: new_tag, args: args) if new_version.blank? false elsif old_hash.blank? if new_tag.blank? && new_version.blank? && new_revision.blank? @@ -191,7 +191,7 @@ module Homebrew and old tag are both #{new_tag}. EOS end - check_new_version(formula, tap_full_name, url: old_url, tag: new_tag, args: args) if new_version.blank? + check_new_version(formula, tap_remote_repo, url: old_url, tag: new_tag, args: args) if new_version.blank? resource_path, forced_version = fetch_resource(formula, new_version, old_url, tag: new_tag) new_revision = Utils.popen_read("git", "-C", resource_path.to_s, "rev-parse", "-q", "--verify", "HEAD") new_revision = new_revision.strip @@ -218,7 +218,7 @@ module Homebrew #{new_url} EOS end - check_new_version(formula, tap_full_name, url: new_url, args: args) if new_version.blank? + check_new_version(formula, tap_remote_repo, url: new_url, args: args) if new_version.blank? resource_path, forced_version = fetch_resource(formula, new_version, new_url) Utils::Tar.validate_file(resource_path) new_hash = resource_path.sha256 @@ -381,7 +381,7 @@ module Homebrew commit_message: "#{formula.name} #{new_formula_version}", previous_branch: previous_branch, tap: formula.tap, - tap_full_name: tap_full_name, + tap_remote_repo: tap_remote_repo, pr_message: pr_message, } GitHub.create_bump_pr(pr_info, args: args) @@ -454,21 +454,21 @@ module Homebrew end end - def check_open_pull_requests(formula, tap_full_name, args:) - GitHub.check_for_duplicate_pull_requests(formula.name, tap_full_name, + def check_open_pull_requests(formula, tap_remote_repo, args:) + GitHub.check_for_duplicate_pull_requests(formula.name, tap_remote_repo, state: "open", file: formula.path.relative_path_from(formula.tap.path).to_s, args: args) end - def check_new_version(formula, tap_full_name, args:, version: nil, url: nil, tag: nil) + def check_new_version(formula, tap_remote_repo, args:, version: nil, url: nil, tag: nil) if version.nil? specs = {} specs[:tag] = tag if tag.present? version = Version.detect(url, **specs) end check_throttle(formula, version) - check_closed_pull_requests(formula, tap_full_name, args: args, version: version) + check_closed_pull_requests(formula, tap_remote_repo, args: args, version: version) end def check_throttle(formula, new_version) @@ -481,9 +481,9 @@ module Homebrew odie "#{formula} should only be updated every #{throttled_rate} releases on multiples of #{throttled_rate}" end - def check_closed_pull_requests(formula, tap_full_name, args:, version:) + def check_closed_pull_requests(formula, tap_remote_repo, args:, version:) # if we haven't already found open requests, try for an exact match across closed requests - GitHub.check_for_duplicate_pull_requests(formula.name, tap_full_name, + GitHub.check_for_duplicate_pull_requests(formula.name, tap_remote_repo, version: version, state: "closed", file: formula.path.relative_path_from(formula.tap.path).to_s, diff --git a/Library/Homebrew/dev-cmd/bump.rb b/Library/Homebrew/dev-cmd/bump.rb index 17655e4b33..ff8451871f 100644 --- a/Library/Homebrew/dev-cmd/bump.rb +++ b/Library/Homebrew/dev-cmd/bump.rb @@ -170,7 +170,8 @@ module Homebrew end def retrieve_pull_requests(formula_or_cask, name) - pull_requests = GitHub.fetch_pull_requests(name, formula_or_cask.tap&.full_name, state: "open") + tap_remote_repo = formula_or_cask.tap&.remote_repo || formula_or_cask.tap&.full_name + pull_requests = GitHub.fetch_pull_requests(name, tap_remote_repo, state: "open") if pull_requests.try(:any?) pull_requests = pull_requests.map { |pr| "#{pr["title"]} (#{Formatter.url(pr["html_url"])})" }.join(", ") end diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index ea4b535a03..07b2b5abcc 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -138,6 +138,14 @@ class Tap @remote ||= path.git_origin end + # The remote repository name of this {Tap}. + # e.g. `user/homebrew-repo` + def remote_repo + raise TapUnavailableError, name unless installed? + + @remote_repo ||= remote&.sub(%r{^https://github\.com/}, "")&.sub(/\.git$/, "") + end + # The default remote path to this {Tap}. sig { returns(String) } def default_remote diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb index c762680710..dee6405fc9 100644 --- a/Library/Homebrew/test/tap_spec.rb +++ b/Library/Homebrew/test/tap_spec.rb @@ -205,6 +205,33 @@ describe Tap do end end + describe "#remote_repo" do + it "returns the remote repository" do + setup_git_repo + + expect(homebrew_foo_tap.remote_repo).to eq("Homebrew/homebrew-foo") + expect { described_class.new("Homebrew", "bar").remote_repo }.to raise_error(TapUnavailableError) + + services_tap = described_class.new("Homebrew", "services") + services_tap.path.mkpath + services_tap.path.cd do + system "git", "init" + system "git", "remote", "add", "origin", "https://github.com/Homebrew/homebrew-bar" + end + expect(services_tap.remote_repo).to eq("Homebrew/homebrew-bar") + end + + it "returns nil if the Tap is not a Git repository" do + expect(homebrew_foo_tap.remote_repo).to be nil + end + + it "returns nil if Git is not available" do + setup_git_repo + allow(Utils::Git).to receive(:available?).and_return(false) + expect(homebrew_foo_tap.remote_repo).to be nil + end + end + specify "Git variant" do touch path/"README" setup_git_repo diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index acef79b932..9ddfdb872b 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -63,8 +63,8 @@ module GitHub end end - def issues_for_formula(name, tap: CoreTap.instance, tap_full_name: tap.full_name, state: nil) - search_issues(name, repo: tap_full_name, state: state, in: "title") + def issues_for_formula(name, tap: CoreTap.instance, tap_remote_repo: tap.full_name, state: nil) + search_issues(name, repo: tap_remote_repo, state: state, in: "title") end def user @@ -410,7 +410,7 @@ module GitHub nil end - def fetch_pull_requests(name, tap_full_name, state: nil, version: nil) + def fetch_pull_requests(name, tap_remote_repo, state: nil, version: nil) if version.present? query = "#{name} #{version}" regex = /(^|\s)#{Regexp.quote(name)}(:|,|\s)(.*\s)?#{Regexp.quote(version)}(:|,|\s|$)/i @@ -418,7 +418,7 @@ module GitHub query = name regex = /(^|\s)#{Regexp.quote(name)}(:|,|\s|$)/i end - issues_for_formula(query, tap_full_name: tap_full_name, state: state).select do |pr| + issues_for_formula(query, tap_remote_repo: tap_remote_repo, state: state).select do |pr| pr["html_url"].include?("/pull/") && regex.match?(pr["title"]) end rescue API::RateLimitExceededError => e @@ -426,9 +426,9 @@ module GitHub [] end - def check_for_duplicate_pull_requests(name, tap_full_name, state:, file:, args:, version: nil) - pull_requests = fetch_pull_requests(name, tap_full_name, state: state, version: version).select do |pr| - pr_files = API.open_rest(url_to("repos", tap_full_name, "pulls", pr["number"], "files")) + def check_for_duplicate_pull_requests(name, tap_remote_repo, state:, file:, args:, version: nil) + pull_requests = fetch_pull_requests(name, tap_remote_repo, state: state, version: version).select do |pr| + pr_files = API.open_rest(url_to("repos", tap_remote_repo, "pulls", pr["number"], "files")) pr_files.any? { |f| f["filename"] == file } end return if pull_requests.blank? @@ -450,10 +450,10 @@ module GitHub end end - def forked_repo_info!(tap_full_name) - response = create_fork(tap_full_name) + def forked_repo_info!(tap_remote_repo) + response = create_fork(tap_remote_repo) # GitHub API responds immediately but fork takes a few seconds to be ready. - sleep 1 until check_fork_exists(tap_full_name) + sleep 1 until check_fork_exists(tap_remote_repo) remote_url = if system("git", "config", "--local", "--get-regexp", "remote\..*\.url", "git@github.com:.*") response.fetch("ssh_url") else @@ -477,7 +477,7 @@ module GitHub branch = info[:branch_name] commit_message = info[:commit_message] previous_branch = info[:previous_branch] || "-" - tap_full_name = info[:tap_full_name] || tap.full_name + tap_remote_repo = info[:tap_remote_repo] || tap.full_name pr_message = info[:pr_message] sourcefile_path.parent.cd do @@ -504,7 +504,7 @@ module GitHub username = tap.user else begin - remote_url, username = forked_repo_info!(tap_full_name) + remote_url, username = forked_repo_info!(tap_remote_repo) rescue *API::ERRORS => e sourcefile_path.atomic_write(old_contents) odie "Unable to fork: #{e.message}!" @@ -538,7 +538,7 @@ module GitHub end begin - url = create_pull_request(tap_full_name, commit_message, + url = create_pull_request(tap_remote_repo, commit_message, "#{username}:#{branch}", remote_branch, pr_message)["html_url"] if args.no_browse? puts url