Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

utils/github: handle non-standard tap remotes #10895

Merged
merged 2 commits into from Mar 23, 2021
Merged

utils/github: handle non-standard tap remotes #10895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e61f61
utils/github: handle non-standard tap remotes
Rylan12 Mar 21, 2021
0b214da
tap: cleanup `remote_repo` method
Rylan12 Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion Library/Homebrew/dev-cmd/bump-cask-pr.rb
View file
Open in desktop
Expand Up @@ -199,7 +199,8 @@ def fetch_cask(contents, version, config: nil)
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)
Expand Down
38 changes: 19 additions & 19 deletions Library/Homebrew/dev-cmd/bump-formula-pr.rb
View file
Open in desktop
Expand Up @@ -85,10 +85,10 @@ def bump_formula_pr_args
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)
Expand All @@ -99,7 +99,7 @@ def use_correct_linux_tap(formula, args:)
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
Expand All @@ -112,7 +112,7 @@ def use_correct_linux_tap(formula, args:)
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
Expand Down Expand Up @@ -145,11 +145,11 @@ def bump_formula_pr
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-") }
Expand All @@ -173,10 +173,10 @@ def bump_formula_pr
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?
Expand All @@ -191,7 +191,7 @@ def bump_formula_pr
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
Expand All @@ -218,7 +218,7 @@ def bump_formula_pr
#{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
Expand Down Expand Up @@ -381,7 +381,7 @@ def bump_formula_pr
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)
Expand Down Expand Up @@ -454,21 +454,21 @@ def formula_version(formula, contents = nil)
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)
Expand All @@ -481,9 +481,9 @@ def check_throttle(formula, new_version)
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,
Expand Down
3 changes: 2 additions & 1 deletion Library/Homebrew/dev-cmd/bump.rb
View file
Open in desktop
Expand Up @@ -170,7 +170,8 @@ def livecheck_result(formula_or_cask)
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
Expand Down
11 changes: 11 additions & 0 deletions Library/Homebrew/tap.rb
View file
Open in desktop
Expand Up @@ -138,6 +138,17 @@ def remote
@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?

return unless remote

@remote_repo ||= remote.delete_prefix("https://github.com/")
.delete_suffix(".git")
end

# The default remote path to this {Tap}.
sig { returns(String) }
def default_remote
Expand Down
27 changes: 27 additions & 0 deletions Library/Homebrew/test/tap_spec.rb
View file
Open in desktop
Expand Up @@ -205,6 +205,33 @@ def setup_completion(link:)
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
Expand Down
26 changes: 13 additions & 13 deletions Library/Homebrew/utils/github.rb
View file
Open in desktop
Expand Up @@ -63,8 +63,8 @@ def search_code(repo: nil, user: "Homebrew", path: ["Formula", "Casks", "."], fi
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
Expand Down Expand Up @@ -410,25 +410,25 @@ def get_repo_license(user, repo)
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
else
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
opoo e.message
[]
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?
Expand All @@ -450,10 +450,10 @@ def check_for_duplicate_pull_requests(name, tap_full_name, state:, file:, args:,
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
Expand All @@ -477,7 +477,7 @@ def create_bump_pr(info, args:)
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
Expand All @@ -504,7 +504,7 @@ def create_bump_pr(info, args:)
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}!"
Expand Down Expand Up @@ -538,7 +538,7 @@ def create_bump_pr(info, args:)
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
Expand Down