utils/github: handle non-standard tap remotes

This commit is contained in:
Rylan Polster 2021-03-21 12:35:45 -04:00
parent 7e2ba95169
commit 4e61f61a20
No known key found for this signature in database
GPG Key ID: 46A744940CFF4D64
6 changed files with 71 additions and 34 deletions

View File

@ -199,7 +199,8 @@ module Homebrew
end end
def check_open_pull_requests(cask, args:) 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", state: "open",
file: cask.sourcefile_path.relative_path_from(cask.tap.path).to_s, file: cask.sourcefile_path.relative_path_from(cask.tap.path).to_s,
args: args) args: args)

View File

@ -85,10 +85,10 @@ module Homebrew
def use_correct_linux_tap(formula, args:) def use_correct_linux_tap(formula, args:)
default_origin_branch = formula.tap.path.git_origin_branch 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") tap_remote_repo = formula.tap.full_name.gsub("linuxbrew", "homebrew")
homebrew_core_url = "https://github.com/#{tap_full_name}" homebrew_core_url = "https://github.com/#{tap_remote_repo}"
homebrew_core_remote = "homebrew" homebrew_core_remote = "homebrew"
previous_branch = formula.tap.path.git_branch || "master" previous_branch = formula.tap.path.git_branch || "master"
formula_path = formula.path.relative_path_from(formula.tap.path) 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 fetch #{homebrew_core_remote} HEAD #{default_origin_branch}"
ohai "git cat-file -e #{full_origin_branch}:#{formula_path}" ohai "git cat-file -e #{full_origin_branch}:#{formula_path}"
ohai "git checkout #{full_origin_branch}" 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 end
formula.tap.path.cd do formula.tap.path.cd do
@ -112,7 +112,7 @@ module Homebrew
if quiet_system "git", "cat-file", "-e", "#{full_origin_branch}:#{formula_path}" if quiet_system "git", "cat-file", "-e", "#{full_origin_branch}:#{formula_path}"
ohai "#{formula.full_name} exists in #{full_origin_branch}." ohai "#{formula.full_name} exists in #{full_origin_branch}."
safe_system "git", "checkout", 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 end
end end
@ -145,11 +145,11 @@ module Homebrew
formula_spec = formula.stable formula_spec = formula.stable
odie "#{formula}: no stable specification found!" if formula_spec.blank? 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) tap_remote_repo, remote, remote_branch, previous_branch = use_correct_linux_tap(formula, args: args)
check_open_pull_requests(formula, tap_full_name, args: args) check_open_pull_requests(formula, tap_remote_repo, args: args)
new_version = args.version 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? opoo "This formula has patches that may be resolved upstream." if formula.patchlist.present?
if formula.resources.any? { |resource| !resource.name.start_with?("homebrew-") } if formula.resources.any? { |resource| !resource.name.start_with?("homebrew-") }
@ -173,10 +173,10 @@ module Homebrew
old_version = old_formula_version.to_s old_version = old_formula_version.to_s
forced_version = new_version.present? forced_version = new_version.present?
new_url_hash = if new_url.present? && new_hash.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 true
elsif new_tag.present? && new_revision.present? 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 false
elsif old_hash.blank? elsif old_hash.blank?
if new_tag.blank? && new_version.blank? && new_revision.blank? if new_tag.blank? && new_version.blank? && new_revision.blank?
@ -191,7 +191,7 @@ module Homebrew
and old tag are both #{new_tag}. and old tag are both #{new_tag}.
EOS EOS
end 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) 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 = Utils.popen_read("git", "-C", resource_path.to_s, "rev-parse", "-q", "--verify", "HEAD")
new_revision = new_revision.strip new_revision = new_revision.strip
@ -218,7 +218,7 @@ module Homebrew
#{new_url} #{new_url}
EOS EOS
end 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) resource_path, forced_version = fetch_resource(formula, new_version, new_url)
Utils::Tar.validate_file(resource_path) Utils::Tar.validate_file(resource_path)
new_hash = resource_path.sha256 new_hash = resource_path.sha256
@ -381,7 +381,7 @@ module Homebrew
commit_message: "#{formula.name} #{new_formula_version}", commit_message: "#{formula.name} #{new_formula_version}",
previous_branch: previous_branch, previous_branch: previous_branch,
tap: formula.tap, tap: formula.tap,
tap_full_name: tap_full_name, tap_remote_repo: tap_remote_repo,
pr_message: pr_message, pr_message: pr_message,
} }
GitHub.create_bump_pr(pr_info, args: args) GitHub.create_bump_pr(pr_info, args: args)
@ -454,21 +454,21 @@ module Homebrew
end end
end end
def check_open_pull_requests(formula, tap_full_name, args:) def check_open_pull_requests(formula, tap_remote_repo, args:)
GitHub.check_for_duplicate_pull_requests(formula.name, tap_full_name, GitHub.check_for_duplicate_pull_requests(formula.name, tap_remote_repo,
state: "open", state: "open",
file: formula.path.relative_path_from(formula.tap.path).to_s, file: formula.path.relative_path_from(formula.tap.path).to_s,
args: args) args: args)
end 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? if version.nil?
specs = {} specs = {}
specs[:tag] = tag if tag.present? specs[:tag] = tag if tag.present?
version = Version.detect(url, **specs) version = Version.detect(url, **specs)
end end
check_throttle(formula, version) 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 end
def check_throttle(formula, new_version) 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}" odie "#{formula} should only be updated every #{throttled_rate} releases on multiples of #{throttled_rate}"
end 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 # 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, version: version,
state: "closed", state: "closed",
file: formula.path.relative_path_from(formula.tap.path).to_s, file: formula.path.relative_path_from(formula.tap.path).to_s,

View File

@ -170,7 +170,8 @@ module Homebrew
end end
def retrieve_pull_requests(formula_or_cask, name) 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?) if pull_requests.try(:any?)
pull_requests = pull_requests.map { |pr| "#{pr["title"]} (#{Formatter.url(pr["html_url"])})" }.join(", ") pull_requests = pull_requests.map { |pr| "#{pr["title"]} (#{Formatter.url(pr["html_url"])})" }.join(", ")
end end

View File

@ -138,6 +138,14 @@ class Tap
@remote ||= path.git_origin @remote ||= path.git_origin
end 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}. # The default remote path to this {Tap}.
sig { returns(String) } sig { returns(String) }
def default_remote def default_remote

View File

@ -205,6 +205,33 @@ describe Tap do
end end
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 specify "Git variant" do
touch path/"README" touch path/"README"
setup_git_repo setup_git_repo

View File

@ -63,8 +63,8 @@ module GitHub
end end
end end
def issues_for_formula(name, tap: CoreTap.instance, tap_full_name: tap.full_name, state: nil) def issues_for_formula(name, tap: CoreTap.instance, tap_remote_repo: tap.full_name, state: nil)
search_issues(name, repo: tap_full_name, state: state, in: "title") search_issues(name, repo: tap_remote_repo, state: state, in: "title")
end end
def user def user
@ -410,7 +410,7 @@ module GitHub
nil nil
end 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? if version.present?
query = "#{name} #{version}" query = "#{name} #{version}"
regex = /(^|\s)#{Regexp.quote(name)}(:|,|\s)(.*\s)?#{Regexp.quote(version)}(:|,|\s|$)/i regex = /(^|\s)#{Regexp.quote(name)}(:|,|\s)(.*\s)?#{Regexp.quote(version)}(:|,|\s|$)/i
@ -418,7 +418,7 @@ module GitHub
query = name query = name
regex = /(^|\s)#{Regexp.quote(name)}(:|,|\s|$)/i regex = /(^|\s)#{Regexp.quote(name)}(:|,|\s|$)/i
end 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"]) pr["html_url"].include?("/pull/") && regex.match?(pr["title"])
end end
rescue API::RateLimitExceededError => e rescue API::RateLimitExceededError => e
@ -426,9 +426,9 @@ module GitHub
[] []
end end
def check_for_duplicate_pull_requests(name, tap_full_name, state:, file:, args:, version: nil) def check_for_duplicate_pull_requests(name, tap_remote_repo, state:, file:, args:, version: nil)
pull_requests = fetch_pull_requests(name, tap_full_name, state: state, version: version).select do |pr| 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_full_name, "pulls", pr["number"], "files")) pr_files = API.open_rest(url_to("repos", tap_remote_repo, "pulls", pr["number"], "files"))
pr_files.any? { |f| f["filename"] == file } pr_files.any? { |f| f["filename"] == file }
end end
return if pull_requests.blank? return if pull_requests.blank?
@ -450,10 +450,10 @@ module GitHub
end end
end end
def forked_repo_info!(tap_full_name) def forked_repo_info!(tap_remote_repo)
response = create_fork(tap_full_name) response = create_fork(tap_remote_repo)
# GitHub API responds immediately but fork takes a few seconds to be ready. # 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:.*") remote_url = if system("git", "config", "--local", "--get-regexp", "remote\..*\.url", "git@github.com:.*")
response.fetch("ssh_url") response.fetch("ssh_url")
else else
@ -477,7 +477,7 @@ module GitHub
branch = info[:branch_name] branch = info[:branch_name]
commit_message = info[:commit_message] commit_message = info[:commit_message]
previous_branch = info[:previous_branch] || "-" 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] pr_message = info[:pr_message]
sourcefile_path.parent.cd do sourcefile_path.parent.cd do
@ -504,7 +504,7 @@ module GitHub
username = tap.user username = tap.user
else else
begin begin
remote_url, username = forked_repo_info!(tap_full_name) remote_url, username = forked_repo_info!(tap_remote_repo)
rescue *API::ERRORS => e rescue *API::ERRORS => e
sourcefile_path.atomic_write(old_contents) sourcefile_path.atomic_write(old_contents)
odie "Unable to fork: #{e.message}!" odie "Unable to fork: #{e.message}!"
@ -538,7 +538,7 @@ module GitHub
end end
begin 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"] "#{username}:#{branch}", remote_branch, pr_message)["html_url"]
if args.no_browse? if args.no_browse?
puts url puts url