new-formula: Don't fail CI instead comment on PR about audit violations

This commit is contained in:
Gautham Goli 2018-04-30 01:22:04 +05:30
parent 03dd26c5fa
commit 26e77dd75c
4 changed files with 73 additions and 17 deletions

View File

@ -117,6 +117,7 @@ module Homebrew
# Check style in a single batch run up front for performance # Check style in a single batch run up front for performance
style_results = check_style_json(files, options) style_results = check_style_json(files, options)
new_formula_problem_lines = []
ff.sort.each do |f| ff.sort.each do |f|
options = { new_formula: new_formula, strict: strict, online: online, only: args.only, except: args.except } options = { new_formula: new_formula, strict: strict, online: online, only: args.only, except: args.except }
options[:style_offenses] = style_results.file_offenses(f.path) options[:style_offenses] = style_results.file_offenses(f.path)
@ -127,7 +128,8 @@ module Homebrew
fa.problems fa.problems
formula_count += 1 formula_count += 1
problem_count += fa.problems.size problem_count += fa.problems.size
problem_lines = fa.problems.map { |p| "* #{p.chomp.gsub("\n", "\n ")}" } problem_lines = format_problem_lines(fa.problems)
new_formula_problem_lines = format_problem_lines(fa.new_formula_problems)
if args.display_filename? if args.display_filename?
puts problem_lines.map { |s| "#{f.path}: #{s}" } puts problem_lines.map { |s| "#{f.path}: #{s}" }
else else
@ -135,9 +137,27 @@ module Homebrew
end end
end end
return if problem_count.zero? create_issue_failed = false
if new_formula && !new_formula_problem_lines.size.zero? && !GitHub.create_issue_no_op?
begin
GitHub.create_issue_comment(new_formula_problem_lines.join("\n"))
rescue *GitHub.api_errors
create_issue_failed = true
end
end
ofail "#{Formatter.pluralize(problem_count, "problem")} in #{Formatter.pluralize(formula_count, "formula")}" if GitHub.create_issue_no_op? || create_issue_failed
problem_count += new_formula_problem_lines.size
puts new_formula_problem_lines.map { |s| " #{s}" }
end
errors_summary = "#{Formatter.pluralize(problem_count, "problem")} in #{Formatter.pluralize(formula_count, "formula")}"
return if problem_count.zero?
ofail errors_summary unless strict
end
def format_problem_lines(problems)
problems.map { |p| "* #{p.chomp.gsub("\n", "\n ")}" }
end end
class FormulaText class FormulaText
@ -184,7 +204,7 @@ module Homebrew
class FormulaAuditor class FormulaAuditor
include FormulaCellarChecks include FormulaCellarChecks
attr_reader :formula, :text, :problems attr_reader :formula, :text, :problems, :new_formula_problems
def initialize(formula, options = {}) def initialize(formula, options = {})
@formula = formula @formula = formula
@ -199,6 +219,7 @@ module Homebrew
# Allow the actual official-ness of a formula to be overridden, for testing purposes # Allow the actual official-ness of a formula to be overridden, for testing purposes
@official_tap = formula.tap&.official? || options[:official_tap] @official_tap = formula.tap&.official? || options[:official_tap]
@problems = [] @problems = []
@new_formula_problems = []
@text = FormulaText.new(formula.path) @text = FormulaText.new(formula.path)
@specs = %w[stable devel head].map { |s| formula.send(s) }.compact @specs = %w[stable devel head].map { |s| formula.send(s) }.compact
end end
@ -206,6 +227,10 @@ module Homebrew
def audit_style def audit_style
return unless @style_offenses return unless @style_offenses
@style_offenses.each do |offense| @style_offenses.each do |offense|
if offense.cop_name.start_with?("NewFormulaAudit")
new_formula_problem offense.to_s(display_cop_name: @display_cop_names)
next
end
problem offense.to_s(display_cop_name: @display_cop_names) problem offense.to_s(display_cop_name: @display_cop_names)
end end
end end
@ -360,7 +385,7 @@ module Homebrew
if @new_formula && dep_f.keg_only_reason && if @new_formula && dep_f.keg_only_reason &&
!["openssl", "apr", "apr-util"].include?(dep.name) && !["openssl", "apr", "apr-util"].include?(dep.name) &&
[:provided_by_macos, :provided_by_osx].include?(dep_f.keg_only_reason.reason) [:provided_by_macos, :provided_by_osx].include?(dep_f.keg_only_reason.reason)
problem "Dependency '#{dep.name}' may be unnecessary as it is provided by macOS; try to build this formula without it." new_formula_problem "Dependency '#{dep.name}' may be unnecessary as it is provided by macOS; try to build this formula without it."
end end
dep.options.each do |opt| dep.options.each do |opt|
@ -475,15 +500,15 @@ module Homebrew
return if metadata.nil? return if metadata.nil?
problem "GitHub fork (not canonical repository)" if metadata["fork"] new_formula_problem "GitHub fork (not canonical repository)" if metadata["fork"]
if formula&.tap&.core_tap? && if formula&.tap&.core_tap? &&
(metadata["forks_count"] < 20) && (metadata["subscribers_count"] < 20) && (metadata["forks_count"] < 20) && (metadata["subscribers_count"] < 20) &&
(metadata["stargazers_count"] < 50) (metadata["stargazers_count"] < 50)
problem "GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)" new_formula_problem "GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)"
end end
return if Date.parse(metadata["created_at"]) <= (Date.today - 30) return if Date.parse(metadata["created_at"]) <= (Date.today - 30)
problem "GitHub repository too new (<30 days old)" new_formula_problem "GitHub repository too new (<30 days old)"
end end
def audit_specs def audit_specs
@ -510,8 +535,7 @@ module Homebrew
end end
next if spec.patches.empty? next if spec.patches.empty?
next unless @new_formula new_formula_problem "New formulae should not require patches to build. Patches should be submitted and accepted upstream first."
problem "New formulae should not require patches to build. Patches should be submitted and accepted upstream first."
end end
%w[Stable Devel].each do |name| %w[Stable Devel].each do |name|
@ -534,7 +558,7 @@ module Homebrew
end end
if @new_formula && formula.head if @new_formula && formula.head
problem "New formulae should not have a HEAD spec" new_formula_problem "New formulae should not have a HEAD spec"
end end
unstable_whitelist = %w[ unstable_whitelist = %w[
@ -815,6 +839,10 @@ module Homebrew
@problems << p @problems << p
end end
def new_formula_problem(p)
@new_formula_problems << p
end
def head_only?(formula) def head_only?(formula)
formula.head && formula.devel.nil? && formula.stable.nil? formula.head && formula.devel.nil? && formula.stable.nil?
end end

View File

@ -131,9 +131,6 @@ module Homebrew
ENV[env] = homebrew_env ENV[env] = homebrew_env
end end
gh_api_errors = [GitHub::AuthenticationFailedError, GitHub::HTTPNotFoundError,
GitHub::RateLimitExceededError, GitHub::Error, JSON::ParserError].freeze
formula = ARGV.formulae.first formula = ARGV.formulae.first
if formula if formula
@ -336,7 +333,7 @@ module Homebrew
response = GitHub.create_fork(formula.tap.full_name) response = GitHub.create_fork(formula.tap.full_name)
# 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 3 sleep 3
rescue *gh_api_errors => e rescue *GitHub.api_errors => e
formula.path.atomic_write(backup_file) unless ARGV.dry_run? formula.path.atomic_write(backup_file) unless ARGV.dry_run?
odie "Unable to fork: #{e.message}!" odie "Unable to fork: #{e.message}!"
end end
@ -372,7 +369,7 @@ module Homebrew
else else
exec_browser url exec_browser url
end end
rescue *gh_api_errors => e rescue *GitHub.api_errors => e
odie "Unable to open pull request: #{e.message}!" odie "Unable to open pull request: #{e.message}!"
end end
end end

View File

@ -281,7 +281,7 @@ module Homebrew
fa.audit_deps fa.audit_deps
end end
its(:problems) { are_expected.to match([/unnecessary/]) } its(:new_formula_problems) { are_expected.to match([/unnecessary/]) }
end end
end end
end end

View File

@ -291,4 +291,35 @@ module GitHub
uri.query = query_string(*queries, **qualifiers) uri.query = query_string(*queries, **qualifiers)
open_api(uri) { |json| json.fetch("items", []) } open_api(uri) { |json| json.fetch("items", []) }
end end
def create_issue_no_op?
!ENV.key?("HOMEBREW_GH_REPO") || !ENV.key?("HOMEBREW_NEW_FORMULA_PULL_REQUEST_URL")
end
def create_issue_comment(body)
repo = ENV["HOMEBREW_GH_REPO"]
pull_request = extract_pull_request_number
url = "https://api.github.com/repos/Homebrew/#{repo}/issues/#{pull_request}/comments"
data = { "body" => body }
return if issue_comment_exists?(repo, pr_number, body)
scopes = CREATE_ISSUE_SCOPES
open_api(url, data: data, scopes: scopes)
end
def extract_pull_request_number
pattern = /#(\d+)$/
ENV["HOMEBREW_NEW_FORMULA_PULL_REQUEST_URL"].match(pattern)[1]
end
def issue_comment_exists?(repo, pull_reqest, body)
url = "https://api.github.com/repos/Homebrew/#{repo}/issues/#{pull_reqest}/comments"
comments = open_api(url)
return unless comments
comments.any? { |comment| comment["body"].eql?(body) }
end
def api_errors
[GitHub::AuthenticationFailedError, GitHub::HTTPNotFoundError,
GitHub::RateLimitExceededError, GitHub::Error, JSON::ParserError].freeze
end
end end