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.

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

bump-*-pr: check existing PRs for exact file match #10251

Merged
merged 2 commits into from Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 10 additions & 17 deletions Library/Homebrew/dev-cmd/bump-cask-pr.rb
Expand Up @@ -67,6 +67,10 @@ def bump_cask_pr
ENV["BROWSER"] = Homebrew::EnvConfig.browser

cask = args.named.to_casks.first

odie "This cask is not in a tap!" if cask.tap.blank?
odie "This cask's tap is not a Git repository!" unless cask.tap.git?

new_version = args.version
new_version = :latest if ["latest", ":latest"].include?(new_version)
new_version = Cask::DSL::Version.new(new_version) if new_version.present?
Expand All @@ -81,12 +85,7 @@ def bump_cask_pr
old_version = cask.version
old_hash = cask.sha256

tap_full_name = cask.tap&.full_name
default_remote_branch = cask.tap.path.git_origin_branch if cask.tap
default_remote_branch ||= "master"
previous_branch = "-"

check_open_pull_requests(cask, tap_full_name, args: args)
check_open_pull_requests(cask, args: args)

old_contents = File.read(cask.sourcefile_path)

Expand Down Expand Up @@ -180,12 +179,9 @@ def bump_cask_pr
pr_info = {
sourcefile_path: cask.sourcefile_path,
old_contents: old_contents,
remote_branch: default_remote_branch,
branch_name: branch_name,
commit_message: commit_message,
previous_branch: previous_branch,
tap: cask.tap,
tap_full_name: tap_full_name,
pr_message: "Created with `brew bump-cask-pr`.",
}
GitHub.create_bump_pr(pr_info, args: args)
Expand All @@ -199,14 +195,11 @@ def fetch_resource(cask, new_version, url, **specs)
resource.fetch
end

def check_open_pull_requests(cask, tap_full_name, args:)
GitHub.check_for_duplicate_pull_requests(cask.token, tap_full_name, state: "open", args: args)
end

def check_closed_pull_requests(cask, tap_full_name, version:, args:)
# if we haven't already found open requests, try for an exact match across closed requests
pr_title = "Update #{cask.token} from #{cask.version} to #{version}"
GitHub.check_for_duplicate_pull_requests(pr_title, tap_full_name, state: "closed", args: args)
def check_open_pull_requests(cask, args:)
GitHub.check_for_duplicate_pull_requests(cask.token, cask.tap.full_name,
state: "open",
file: cask.sourcefile_path.relative_path_from(cask.tap.path).to_s,
args: args)
end

def run_cask_audit(cask, old_contents, args:)
Expand Down
16 changes: 12 additions & 4 deletions Library/Homebrew/dev-cmd/bump-formula-pr.rb
Expand Up @@ -84,9 +84,9 @@ def bump_formula_pr_args
end

def use_correct_linux_tap(formula, args:)
default_origin_branch = formula.tap.path.git_origin_branch if formula.tap
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.full_name, "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}"
Expand Down Expand Up @@ -139,6 +139,8 @@ def bump_formula_pr
raise FormulaUnspecifiedError if formula.blank?

odie "This formula is disabled!" if formula.disabled?
odie "This formula is not in a tap!" if formula.tap.blank?
odie "This formula's tap is not a Git repository!" unless formula.tap.git?

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)
Expand Down Expand Up @@ -460,7 +462,10 @@ def formula_version(formula, spec, contents = nil)
end

def check_open_pull_requests(formula, tap_full_name, args:)
GitHub.check_for_duplicate_pull_requests(formula.name, tap_full_name, state: "open", args: args)
GitHub.check_for_duplicate_pull_requests(formula.name, tap_full_name,
state: "open",
file: formula.path.relative_path_from(formula.tap.path).to_s,
MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved
args: args)
end

def check_closed_pull_requests(formula, tap_full_name, args:, version: nil, url: nil, tag: nil)
Expand All @@ -470,7 +475,10 @@ def check_closed_pull_requests(formula, tap_full_name, args:, version: nil, url:
version = Version.detect(url, **specs)
end
# if we haven't already found open requests, try for an exact match across closed requests
GitHub.check_for_duplicate_pull_requests("#{formula.name} #{version}", tap_full_name, state: "closed", args: args)
GitHub.check_for_duplicate_pull_requests("#{formula.name} #{version}", tap_full_name,
state: "closed",
file: formula.path.relative_path_from(formula.tap.path).to_s,
args: args)
end

def alias_update_pair(formula, new_formula_version)
Expand Down
14 changes: 9 additions & 5 deletions Library/Homebrew/utils/github.rb
Expand Up @@ -628,8 +628,12 @@ def fetch_pull_requests(query, tap_full_name, state: nil)
[]
end

def check_for_duplicate_pull_requests(query, tap_full_name, state:, args:)
def check_for_duplicate_pull_requests(query, tap_full_name, state:, file:, args:)
pull_requests = fetch_pull_requests(query, tap_full_name, state: state)
pull_requests.select! do |pr|
pr_files = open_api(url_to("repos", tap_full_name, "pulls", pr["number"], "files"))
pr_files.any? { |f| f["filename"] == file }
end
return if pull_requests.blank?

duplicates_message = <<~EOS
Expand Down Expand Up @@ -667,16 +671,16 @@ def forked_repo_info!(tap_full_name)
end

def create_bump_pr(info, args:)
tap = info[:tap]
sourcefile_path = info[:sourcefile_path]
old_contents = info[:old_contents]
additional_files = info[:additional_files] || []
remote = info[:remote] || "origin"
remote_branch = info[:remote_branch]
remote_branch = info[:remote_branch] || tap.path.git_origin_branch
branch = info[:branch_name]
commit_message = info[:commit_message]
previous_branch = info[:previous_branch]
tap = info[:tap]
tap_full_name = info[:tap_full_name]
previous_branch = info[:previous_branch] || "-"
tap_full_name = info[:tap_full_name] || tap.full_name
pr_message = info[:pr_message]

sourcefile_path.parent.cd do
Expand Down