admin/brew
1
0
Fork 0
Code Issues Packages Projects Releases 6 Wiki Activity

Merge pull request #2976 from GauthamGoli/audit_external_patches_rubocop

Browse Source 
audit: Port patches audit code to a rubocop and add tests
This commit is contained in:
Mike McQuaid 2017-08-05 17:39:40 +01:00 committed by GitHub
commit 8405158d8d
4 changed files with 88 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
Library/Homebrew/dev-cmd/audit.rb
View File

@ -630,7 +630,6 @@ class FormulaAuditor
end
next if spec.patches.empty?
spec.patches.each { |p| patch_problems(p) if p.external? }
next unless @new_formula
problem "New formulae should not require patches to build. Patches should be submitted and accepted upstream first."
end
@ -786,36 +785,6 @@ class FormulaAuditor
end
end
def patch_problems(patch)
case patch.url
when %r{https?://github\.com/.+/.+/(?:commit|pull)/[a-fA-F0-9]*.(?:patch|diff)}
unless patch.url =~ /\?full_index=\w+$/
problem <<-EOS.undent
GitHub patches should use the full_index parameter:
#{patch.url}?full_index=1
EOS
end
when /raw\.github\.com/, %r{gist\.github\.com/raw}, %r{gist\.github\.com/.+/raw},
%r{gist\.githubusercontent\.com/.+/raw}
unless patch.url =~ /[a-fA-F0-9]{40}/
problem "GitHub/Gist patches should specify a revision:\n#{patch.url}"
end
when %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}
problem <<-EOS.undent
use GitHub pull request URLs:
https://github.com/#{Regexp.last_match(1)}/#{Regexp.last_match(2)}/pull/#{Regexp.last_match(3)}.patch?full_index=1
Rather than patch-diff:
#{patch.url}
EOS
when %r{macports/trunk}
problem "MacPorts patches should specify a revision instead of trunk:\n#{patch.url}"
when %r{^http://trac\.macports\.org}
problem "Patches from MacPorts Trac should be https://, not http:\n#{patch.url}"
when %r{^http://bugs\.debian\.org}
problem "Patches from Debian should be https://, not http:\n#{patch.url}"
end
end
def audit_text
bin_names = Set.new
bin_names << formula.name

2
Library/Homebrew/rubocops.rb
View File

@ -6,7 +6,7 @@ require_relative "./rubocops/homepage_cop"
require_relative "./rubocops/text_cop"
require_relative "./rubocops/caveats_cop"
require_relative "./rubocops/checksum_cop"
require_relative "./rubocops/legacy_patches_cop"
require_relative "./rubocops/patches_cop"
require_relative "./rubocops/conflicts_cop"
require_relative "./rubocops/options_cop"
require_relative "./rubocops/urls_cop"

15
Library/Homebrew/rubocops/legacy_patches_cop.rb → Library/Homebrew/rubocops/patches_cop.rb
View File

@ -4,9 +4,16 @@ require_relative "../extend/string"
module RuboCop
module Cop
module FormulaAudit
# This cop checks for and audits legacy patches in Formulae
class LegacyPatches < FormulaCop
# This cop audits patches in Formulae
class Patches < FormulaCop
def audit_formula(_node, _class_node, _parent_class_node, body)
external_patches = find_all_blocks(body, :patch)
external_patches.each do |patch_block|
url_node = find_every_method_call_by_name(patch_block, :url).first
url_string = parameters(url_node).first
patch_problems(url_string)
end
patches_node = find_method_def(body, :patches)
return if patches_node.nil?
legacy_patches = find_strings(patches_node)
@ -14,6 +21,8 @@ module RuboCop
legacy_patches.each { |p| patch_problems(p) }
end
private
def patch_problems(patch)
patch_url = string_content(patch)
gh_patch_patterns = Regexp.union([%r{/raw\.github\.com/},
@ -30,7 +39,7 @@ module RuboCop
if match_obj = regex_match_group(patch, gh_patch_diff_pattern)
problem <<-EOS.undent
use GitHub pull request URLs:
https://github.com/#{match_obj[1]}/#{match_obj[2]}/pull/#{match_ojb[3]}.patch
https://github.com/#{match_obj[1]}/#{match_obj[2]}/pull/#{match_obj[3]}.patch
Rather than patch-diff:
#{patch_url}
EOS

77
Library/Homebrew/test/rubocops/legacy_patches_cop_spec.rb → Library/Homebrew/test/rubocops/patches_cop_spec.rb
View File

@ -1,9 +1,9 @@
require "rubocop"
require "rubocop/rspec/support"
require_relative "../../extend/string"
require_relative "../../rubocops/legacy_patches_cop"
require_relative "../../rubocops/patches_cop"
describe RuboCop::Cop::FormulaAudit::LegacyPatches do
describe RuboCop::Cop::FormulaAudit::Patches do
subject(:cop) { described_class.new }
context "When auditing legacy patches" do
@ -47,6 +47,7 @@ describe RuboCop::Cop::FormulaAudit::LegacyPatches do
"https://mirrors.ustc.edu.cn/macports/trunk/",
"http://trac.macports.org/export/102865/trunk/dports/mail/uudeview/files/inews.c.patch",
"http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=patch-libunac1.txt;att=1;bug=623340",
"https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch",
]
patch_urls.each do |patch_url|
source = <<-EOS.undent
@ -84,6 +85,15 @@ describe RuboCop::Cop::FormulaAudit::LegacyPatches do
line: 5,
column: 5,
source: source }]
elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}
expected_offenses = [{ message: "use GitHub pull request URLs:\n"\
" https://github.com/foo/foo-bar/pull/100.patch\n"\
"Rather than patch-diff:\n"\
" https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch\n",
severity: :convention,
line: 5,
column: 5,
source: source }]
end
expected_offenses.zip([cop.offenses.last]).each do |expected, actual|
expect_offense(expected, actual)
@ -125,4 +135,67 @@ describe RuboCop::Cop::FormulaAudit::LegacyPatches do
end
end
end
context "When auditing external patches" do
it "Patch URLs" do
patch_urls = [
"https://raw.github.com/mogaal/sendemail",
"https://mirrors.ustc.edu.cn/macports/trunk/",
"http://trac.macports.org/export/102865/trunk/dports/mail/uudeview/files/inews.c.patch",
"http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=patch-libunac1.txt;att=1;bug=623340",
"https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch",
]
patch_urls.each do |patch_url|
source = <<-EOS.undent
class Foo < Formula
homepage "ftp://example.com/foo"
url "http://example.com/foo-1.0.tgz"
patch do
url "#{patch_url}"
sha256 "63376b8fdd6613a91976106d9376069274191860cd58f039b29ff16de1925621"
end
end
EOS
inspect_source(cop, source)
if patch_url =~ %r{/raw\.github\.com/}
expected_offenses = [{ message: "GitHub/Gist patches should specify a revision:\n#{patch_url}",
severity: :convention,
line: 5,
column: 16,
source: source }]
elsif patch_url =~ %r{macports/trunk}
expected_offenses = [{ message: "MacPorts patches should specify a revision instead of trunk:\n#{patch_url}",
severity: :convention,
line: 5,
column: 37,
source: source }]
elsif patch_url =~ %r{^http://trac\.macports\.org}
expected_offenses = [{ message: "Patches from MacPorts Trac should be https://, not http:\n#{patch_url}",
severity: :convention,
line: 5,
column: 9,
source: source }]
elsif patch_url =~ %r{^http://bugs\.debian\.org}
expected_offenses = [{ message: "Patches from Debian should be https://, not http:\n#{patch_url}",
severity: :convention,
line: 5,
column: 9,
source: source }]
elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}
expected_offenses = [{ message: "use GitHub pull request URLs:\n"\
" https://github.com/foo/foo-bar/pull/100.patch\n"\
"Rather than patch-diff:\n"\
" https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch\n",
severity: :convention,
line: 5,
column: 9,
source: source }]
end
expected_offenses.zip([cop.offenses.last]).each do |expected, actual|
expect_offense(expected, actual)
end
end
end
end
end