From 374c3985d68c98786cfe0ec5868fca438caab6ec Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Mon, 4 Jul 2022 19:34:30 +0200 Subject: [PATCH] pr-pull: check for conflicts with long running builds This change will prevent us having to run some long running builds multiple times and to rely on luck to get things merged without conflicts. The check takes less than 30 secondes on my local setup. --- Library/Homebrew/dev-cmd/pr-pull.rb | 41 +++++++++++++++++++++++++++++ Library/Homebrew/utils/github.rb | 9 +++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 1c7fd73ba9..faee988bd4 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -368,6 +368,45 @@ module Homebrew end end + def pr_check_conflicts(name, tap_remote_repo, pr) + hash_template = proc.new { |h, k| h[k] = [] } + long_build_pr_files = GitHub.search_issues( + "org:#{name}", repo: tap_remote_repo, state: "open", label: "\"no long build conflict\"" + ).each_with_object(Hash.new(hash_template)) do |long_build_pr, hash| + number = long_build_pr["number"] + GitHub.get_pull_request_changed_files(name, tap_remote_repo, number).each do |file| + key = file["filename"] + hash[key] << number + end + end + + this_pr_files = GitHub.get_pull_request_changed_files(name, tap_remote_repo, pr) + + conflicts = this_pr_files.each_with_object(Hash.new(hash_template)) do |file, hash| + filename = file["filename"] + next unless long_build_pr_files.key?(filename) + + long_build_pr_files[filename].each do |pr_number| + key = "#{tap_remote_repo}/pull/#{pr_number}" + hash[key] << filename + end + end + return if conflicts.blank? + + # Raise an error, display the conflicting PR. For example: + # Error: You are trying to merge a pull request that conflicts with a long running build in: + # { + # "homebrew-core/pull/98809": [ + # "Formula/icu4c.rb", + # "Formula/node@10.rb" + # ] + # } + odie <<~EOS + You are trying to merge a pull request that conflicts with a long running build in: + #{JSON.pretty_generate(conflicts)} + EOS + end + def pr_pull args = pr_pull_args.parse @@ -397,6 +436,8 @@ module Homebrew opoo "Current branch is #{tap.path.git_branch}: do you need to pull inside #{tap.path.git_origin_branch}?" end + pr_check_conflicts(user, repo, pr) + ohai "Fetching #{tap} pull request ##{pr}" Dir.mktmpdir pr do |dir| cd dir do diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index 072ea202c3..f75af77fa1 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -479,8 +479,9 @@ module GitHub 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 } + get_pull_request_changed_files( + name, tap_remote_repo, pr["number"] + ).any? { |f| f["filename"] == file } end return if pull_requests.blank? @@ -501,6 +502,10 @@ module GitHub end end + def get_pull_request_changed_files(name, tap_remote_repo, pr) + API.open_rest(url_to("repos", name, tap_remote_repo, "pulls", pr, "files")) + end + def forked_repo_info!(tap_remote_repo, org: nil) response = create_fork(tap_remote_repo, org: org) # GitHub API responds immediately but fork takes a few seconds to be ready.