mirror of
https://github.com/Homebrew/brew.git
synced 2025-07-14 16:09:03 +08:00
50d64da1ec
I don’t know how maintainers are going to feel about this, to be honest. If it’s too clunky, perhaps we could externalise the entire two main blocks here and then require that file into the audit instead? Basically, I’m pushing changes here to better detect a wide-array of SSL/TLS available links that either have no auto-redirect in place or is a common linking error in formulae. I haven’t spotted any false positives yet, but obviously, feel free to try and break the changes and I’ll fix as necessary ;). IMO, this would allow us gradual updates without having to mass-update everything at once and stress the bot and inform users they have hundreds of updates pending when really it’s just style/basic changes. Closes Homebrew/homebrew#35551. Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
792 lines
24 KiB
Ruby
792 lines
24 KiB
Ruby
require 'formula'
|
|
require 'utils'
|
|
require 'extend/ENV'
|
|
require 'formula_cellar_checks'
|
|
|
|
module Homebrew
|
|
def audit
|
|
formula_count = 0
|
|
problem_count = 0
|
|
|
|
strict = ARGV.include? "--strict"
|
|
if strict && ARGV.formulae.any? && MacOS.version >= :mavericks
|
|
require "cmd/style"
|
|
ohai "brew style #{ARGV.formulae.join " "}"
|
|
style
|
|
end
|
|
|
|
ENV.activate_extensions!
|
|
ENV.setup_build_environment
|
|
|
|
ff = if ARGV.named.empty?
|
|
Formula
|
|
else
|
|
ARGV.formulae
|
|
end
|
|
|
|
output_header = !strict
|
|
|
|
ff.each do |f|
|
|
fa = FormulaAuditor.new(f, :strict => strict)
|
|
fa.audit
|
|
|
|
unless fa.problems.empty?
|
|
unless output_header
|
|
puts
|
|
ohai "audit problems"
|
|
output_header = true
|
|
end
|
|
|
|
formula_count += 1
|
|
problem_count += fa.problems.size
|
|
puts "#{f.name}:", fa.problems.map { |p| " * #{p}" }, ""
|
|
end
|
|
end
|
|
|
|
unless problem_count.zero?
|
|
ofail "#{problem_count} problems in #{formula_count} formulae"
|
|
end
|
|
end
|
|
end
|
|
|
|
class FormulaText
|
|
def initialize path
|
|
@text = path.open("rb", &:read)
|
|
end
|
|
|
|
def without_patch
|
|
@text.split("\n__END__").first
|
|
end
|
|
|
|
def has_DATA?
|
|
/^[^#]*\bDATA\b/ =~ @text
|
|
end
|
|
|
|
def has_END?
|
|
/^__END__$/ =~ @text
|
|
end
|
|
|
|
def has_trailing_newline?
|
|
/\Z\n/ =~ @text
|
|
end
|
|
end
|
|
|
|
class FormulaAuditor
|
|
include FormulaCellarChecks
|
|
|
|
attr_reader :formula, :text, :problems
|
|
|
|
BUILD_TIME_DEPS = %W[
|
|
autoconf
|
|
automake
|
|
boost-build
|
|
bsdmake
|
|
cmake
|
|
imake
|
|
intltool
|
|
libtool
|
|
pkg-config
|
|
scons
|
|
smake
|
|
swig
|
|
]
|
|
|
|
FILEUTILS_METHODS = FileUtils.singleton_methods(false).join "|"
|
|
|
|
def initialize(formula, options={})
|
|
@formula = formula
|
|
@strict = !!options[:strict]
|
|
@problems = []
|
|
@text = FormulaText.new(formula.path)
|
|
@specs = %w{stable devel head}.map { |s| formula.send(s) }.compact
|
|
end
|
|
|
|
def audit_file
|
|
unless formula.path.stat.mode == 0100644
|
|
problem "Incorrect file permissions: chmod 644 #{formula.path}"
|
|
end
|
|
|
|
if text.has_DATA? and not text.has_END?
|
|
problem "'DATA' was found, but no '__END__'"
|
|
end
|
|
|
|
if text.has_END? and not text.has_DATA?
|
|
problem "'__END__' was found, but 'DATA' is not used"
|
|
end
|
|
|
|
unless text.has_trailing_newline?
|
|
problem "File should end with a newline"
|
|
end
|
|
end
|
|
|
|
def audit_class
|
|
if @strict
|
|
unless formula.test_defined?
|
|
problem "A `test do` test block should be added"
|
|
end
|
|
end
|
|
end
|
|
|
|
@@aliases ||= Formula.aliases
|
|
|
|
def audit_deps
|
|
@specs.each do |spec|
|
|
# Check for things we don't like to depend on.
|
|
# We allow non-Homebrew installs whenever possible.
|
|
spec.deps.each do |dep|
|
|
begin
|
|
dep_f = dep.to_formula
|
|
rescue TapFormulaUnavailableError
|
|
# Don't complain about missing cross-tap dependencies
|
|
next
|
|
rescue FormulaUnavailableError
|
|
problem "Can't find dependency #{dep.name.inspect}."
|
|
next
|
|
end
|
|
|
|
if @@aliases.include?(dep.name)
|
|
problem "Dependency '#{dep.name}' is an alias; use the canonical name '#{dep.to_formula.name}'."
|
|
end
|
|
|
|
dep.options.reject do |opt|
|
|
next true if dep_f.option_defined?(opt)
|
|
dep_f.requirements.detect do |r|
|
|
if r.recommended?
|
|
opt.name == "with-#{r.name}"
|
|
elsif r.optional?
|
|
opt.name == "without-#{r.name}"
|
|
end
|
|
end
|
|
end.each do |opt|
|
|
problem "Dependency #{dep} does not define option #{opt.name.inspect}"
|
|
end
|
|
|
|
case dep.name
|
|
when *BUILD_TIME_DEPS
|
|
next if dep.build? or dep.run?
|
|
problem %{#{dep} dependency should be "depends_on '#{dep}' => :build"}
|
|
when "git", "ruby", "mercurial"
|
|
problem "Don't use #{dep} as a dependency. We allow non-Homebrew #{dep} installations."
|
|
when 'gfortran'
|
|
problem "Use `depends_on :fortran` instead of `depends_on 'gfortran'`"
|
|
when 'open-mpi', 'mpich2'
|
|
problem <<-EOS.undent
|
|
There are multiple conflicting ways to install MPI. Use an MPIDependency:
|
|
depends_on :mpi => [<lang list>]
|
|
Where <lang list> is a comma delimited list that can include:
|
|
:cc, :cxx, :f77, :f90
|
|
EOS
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
def audit_conflicts
|
|
formula.conflicts.each do |c|
|
|
begin
|
|
Formulary.factory(c.name)
|
|
rescue FormulaUnavailableError
|
|
problem "Can't find conflicting formula #{c.name.inspect}."
|
|
end
|
|
end
|
|
end
|
|
|
|
def audit_options
|
|
formula.options.each do |o|
|
|
next unless @strict
|
|
if o.name !~ /with(out)?-/ && o.name != "c++11" && o.name != "universal" && o.name != "32-bit"
|
|
problem "Options should begin with with/without. Migrate '--#{o.name}' with `deprecated_option`."
|
|
end
|
|
end
|
|
end
|
|
|
|
def audit_urls
|
|
homepage = formula.homepage
|
|
|
|
unless homepage =~ %r[^https?://]
|
|
problem "The homepage should start with http or https (URL is #{homepage})."
|
|
end
|
|
|
|
# Check for http:// GitHub homepage urls, https:// is preferred.
|
|
# Note: only check homepages that are repo pages, not *.github.com hosts
|
|
if homepage =~ %r[^http://github\.com/]
|
|
problem "Use https:// URLs for homepages on GitHub (URL is #{homepage})."
|
|
end
|
|
|
|
# Google Code homepages should end in a slash
|
|
if homepage =~ %r[^https?://code\.google\.com/p/[^/]+[^/]$]
|
|
problem "Google Code homepage should end with a slash (URL is #{homepage})."
|
|
end
|
|
|
|
# Automatic redirect exists, but this is another hugely common error.
|
|
if homepage =~ %r[^http://code\.google\.com/]
|
|
problem "Google Code homepages should be https:// links (URL is #{homepage})."
|
|
end
|
|
|
|
# GNU has full SSL/TLS support but no auto-redirect.
|
|
if homepage =~ %r[^http://www\.gnu\.org/]
|
|
problem "GNU homepages should be https:// links (URL is #{homepage})."
|
|
end
|
|
|
|
# Savannah has full SSL/TLS support but no auto-redirect.
|
|
# Doesn't apply to the download links (boo), only the homepage.
|
|
if homepage =~ %r[^http://savannah\.nongnu\.org/]
|
|
problem "Savannah homepages should be https:// links (URL is #{homepage})."
|
|
end
|
|
|
|
# There's an auto-redirect here, but this mistake is incredibly common too.
|
|
if homepage =~ %r[^http://packages\.debian\.org]
|
|
problem "Debian homepage should be https:// links (URL is #{homepage})."
|
|
end
|
|
|
|
if homepage =~ %r[^http://((?:trac|tools|www)\.)?ietf\.org]
|
|
problem "ietf homepages should be https:// links (URL is #{homepage})."
|
|
end
|
|
|
|
# There's an auto-redirect here, but this mistake is incredibly common too.
|
|
# Only applies to the homepage and subdomains for now, not the FTP links.
|
|
if homepage =~ %r[^http://((?:build|cloud|developer|download|extensions|git|glade|help|library|live|nagios|news|people|projects|rt|static|wiki|www)\.)?gnome\.org]
|
|
problem "Gnome homepages should be https:// links (URL is #{homepage})."
|
|
end
|
|
|
|
urls = @specs.map(&:url)
|
|
|
|
# Check GNU urls; doesn't apply to mirrors
|
|
urls.grep(%r[^(?:https?|ftp)://(?!alpha).+/gnu/]) do |u|
|
|
problem "\"ftpmirror.gnu.org\" is preferred for GNU software (url is #{u})."
|
|
end
|
|
|
|
# the rest of the checks apply to mirrors as well.
|
|
urls.concat(@specs.map(&:mirrors).flatten)
|
|
|
|
# Check a variety of SSL/TLS links that don't consistently auto-redirect
|
|
# or are overly common errors that need to be reduced & fixed over time.
|
|
urls.each do |p|
|
|
# Skip the main url link, as it can't be made SSL/TLS yet.
|
|
next if p =~ %r[/ftpmirror\.gnu\.org]
|
|
|
|
case p
|
|
when %r[^http://ftp\.gnu\.org/]
|
|
problem "ftp.gnu.org urls should be https://, not http:// (url is #{p})."
|
|
when %r[^http://code\.google\.com/]
|
|
problem "code.google.com urls should be https://, not http (url is #{p})."
|
|
when %r[^http://fossies\.org/]
|
|
problem "Fossies urls should be https://, not http (url is #{p})."
|
|
when %r[^http://mirrors\.kernel\.org/]
|
|
problem "mirrors.kernel urls should be https://, not http (url is #{p})."
|
|
when %r[^http://tools\.ietf\.org/]
|
|
problem "ietf urls should be https://, not http (url is #{p})."
|
|
end
|
|
end
|
|
|
|
# Check SourceForge urls
|
|
urls.each do |p|
|
|
# Skip if the URL looks like a SVN repo
|
|
next if p =~ %r[/svnroot/]
|
|
next if p =~ %r[svn\.sourceforge]
|
|
|
|
# Is it a sourceforge http(s) URL?
|
|
next unless p =~ %r[^https?://.*\b(sourceforge|sf)\.(com|net)]
|
|
|
|
if p =~ /(\?|&)use_mirror=/
|
|
problem "Don't use #{$1}use_mirror in SourceForge urls (url is #{p})."
|
|
end
|
|
|
|
if p =~ /\/download$/
|
|
problem "Don't use /download in SourceForge urls (url is #{p})."
|
|
end
|
|
|
|
if p =~ %r[^https?://sourceforge\.]
|
|
problem "Use http://downloads.sourceforge.net to get geolocation (url is #{p})."
|
|
end
|
|
|
|
if p =~ %r[^https?://prdownloads\.]
|
|
problem "Don't use prdownloads in SourceForge urls (url is #{p}).\n" +
|
|
"\tSee: http://librelist.com/browser/homebrew/2011/1/12/prdownloads-is-bad/"
|
|
end
|
|
|
|
if p =~ %r[^http://\w+\.dl\.]
|
|
problem "Don't use specific dl mirrors in SourceForge urls (url is #{p})."
|
|
end
|
|
|
|
if p.start_with? "http://downloads"
|
|
problem "Use https:// URLs for downloads from SourceForge (url is #{p})."
|
|
end
|
|
end
|
|
|
|
# Check for Google Code download urls, https:// is preferred
|
|
urls.grep(%r[^http://.*\.googlecode\.com/files.*]) do |u|
|
|
problem "Use https:// URLs for downloads from Google Code (url is #{u})."
|
|
end
|
|
|
|
# Check for new-url Google Code download urls, https:// is preferred
|
|
urls.grep(%r[^http://code\.google\.com/]) do |u|
|
|
problem "Use https:// URLs for downloads from code.google (url is #{u})."
|
|
end
|
|
|
|
# Check for git:// GitHub repo urls, https:// is preferred.
|
|
urls.grep(%r[^git://[^/]*github\.com/]) do |u|
|
|
problem "Use https:// URLs for accessing GitHub repositories (url is #{u})."
|
|
end
|
|
|
|
# Check for git:// Gitorious repo urls, https:// is preferred.
|
|
urls.grep(%r[^git://[^/]*gitorious\.org/]) do |u|
|
|
problem "Use https:// URLs for accessing Gitorious repositories (url is #{u})."
|
|
end
|
|
|
|
# Check for http:// GitHub repo urls, https:// is preferred.
|
|
urls.grep(%r[^http://github\.com/.*\.git$]) do |u|
|
|
problem "Use https:// URLs for accessing GitHub repositories (url is #{u})."
|
|
end
|
|
|
|
# Use new-style archive downloads
|
|
urls.select { |u| u =~ %r[https://.*github.*/(?:tar|zip)ball/] && u !~ %r[\.git$] }.each do |u|
|
|
problem "Use /archive/ URLs for GitHub tarballs (url is #{u})."
|
|
end
|
|
|
|
# Don't use GitHub .zip files
|
|
urls.select { |u| u =~ %r[https://.*github.*/(archive|releases)/.*\.zip$] && u !~ %r[releases/download] }.each do |u|
|
|
problem "Use GitHub tarballs rather than zipballs (url is #{u})."
|
|
end
|
|
end
|
|
|
|
def audit_specs
|
|
if head_only?(formula) && formula.tap != "homebrew/homebrew-head-only"
|
|
problem "Head-only (no stable download)"
|
|
end
|
|
|
|
%w[Stable Devel HEAD].each do |name|
|
|
next unless spec = formula.send(name.downcase)
|
|
|
|
ra = ResourceAuditor.new(spec).audit
|
|
problems.concat ra.problems.map { |problem| "#{name}: #{problem}" }
|
|
|
|
spec.resources.each_value do |resource|
|
|
ra = ResourceAuditor.new(resource).audit
|
|
problems.concat ra.problems.map { |problem|
|
|
"#{name} resource #{resource.name.inspect}: #{problem}"
|
|
}
|
|
end
|
|
|
|
spec.patches.select(&:external?).each { |p| audit_patch(p) }
|
|
end
|
|
|
|
if formula.stable && formula.devel
|
|
if formula.devel.version < formula.stable.version
|
|
problem "devel version #{formula.devel.version} is older than stable version #{formula.stable.version}"
|
|
elsif formula.devel.version == formula.stable.version
|
|
problem "stable and devel versions are identical"
|
|
end
|
|
end
|
|
end
|
|
|
|
def audit_patches
|
|
legacy_patches = Patch.normalize_legacy_patches(formula.patches).grep(LegacyPatch)
|
|
if legacy_patches.any?
|
|
problem "Use the patch DSL instead of defining a 'patches' method"
|
|
legacy_patches.each { |p| audit_patch(p) }
|
|
end
|
|
end
|
|
|
|
def audit_patch(patch)
|
|
case patch.url
|
|
when %r[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[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}"
|
|
when %r[^https?://github\.com/.*commit.*\.patch$]
|
|
problem "GitHub appends a git version to patches; use .diff instead."
|
|
end
|
|
end
|
|
|
|
def audit_text
|
|
if text =~ /system\s+['"]scons/
|
|
problem "use \"scons *args\" instead of \"system 'scons', *args\""
|
|
end
|
|
|
|
if text =~ /system\s+['"]xcodebuild/
|
|
problem %{use "xcodebuild *args" instead of "system 'xcodebuild', *args"}
|
|
end
|
|
|
|
if text =~ /xcodebuild[ (]["'*]/ && text !~ /SYMROOT=/
|
|
problem %{xcodebuild should be passed an explicit "SYMROOT"}
|
|
end
|
|
|
|
if text =~ /Formula\.factory\(/
|
|
problem "\"Formula.factory(name)\" is deprecated in favor of \"Formula[name]\""
|
|
end
|
|
end
|
|
|
|
def audit_line(line, lineno)
|
|
if line =~ /<(Formula|AmazonWebServicesFormula|ScriptFileFormula|GithubGistFormula)/
|
|
problem "Use a space in class inheritance: class Foo < #{$1}"
|
|
end
|
|
|
|
# Commented-out cmake support from default template
|
|
if line =~ /# system "cmake/
|
|
problem "Commented cmake call found"
|
|
end
|
|
|
|
# Comments from default template
|
|
if line =~ /# PLEASE REMOVE/
|
|
problem "Please remove default template comments"
|
|
end
|
|
if line =~ /# if this fails, try separate make\/make install steps/
|
|
problem "Please remove default template comments"
|
|
end
|
|
if line =~ /# if your formula requires any X11\/XQuartz components/
|
|
problem "Please remove default template comments"
|
|
end
|
|
if line =~ /# if your formula fails when building in parallel/
|
|
problem "Please remove default template comments"
|
|
end
|
|
if line =~ /# Remove unrecognized options if warned by configure/
|
|
problem "Please remove default template comments"
|
|
end
|
|
|
|
# FileUtils is included in Formula
|
|
# encfs modifies a file with this name, so check for some leading characters
|
|
if line =~ /[^'"\/]FileUtils\.(\w+)/
|
|
problem "Don't need 'FileUtils.' before #{$1}."
|
|
end
|
|
|
|
# Check for long inreplace block vars
|
|
if line =~ /inreplace .* do \|(.{2,})\|/
|
|
problem "\"inreplace <filenames> do |s|\" is preferred over \"|#{$1}|\"."
|
|
end
|
|
|
|
# Check for string interpolation of single values.
|
|
if line =~ /(system|inreplace|gsub!|change_make_var!).*[ ,]"#\{([\w.]+)\}"/
|
|
problem "Don't need to interpolate \"#{$2}\" with #{$1}"
|
|
end
|
|
|
|
# Check for string concatenation; prefer interpolation
|
|
if line =~ /(#\{\w+\s*\+\s*['"][^}]+\})/
|
|
problem "Try not to concatenate paths in string interpolation:\n #{$1}"
|
|
end
|
|
|
|
# Prefer formula path shortcuts in Pathname+
|
|
if line =~ %r{\(\s*(prefix\s*\+\s*(['"])(bin|include|libexec|lib|sbin|share|Frameworks)[/'"])}
|
|
problem "\"(#{$1}...#{$2})\" should be \"(#{$3.downcase}+...)\""
|
|
end
|
|
|
|
if line =~ %r[((man)\s*\+\s*(['"])(man[1-8])(['"]))]
|
|
problem "\"#{$1}\" should be \"#{$4}\""
|
|
end
|
|
|
|
# Prefer formula path shortcuts in strings
|
|
if line =~ %r[(\#\{prefix\}/(bin|include|libexec|lib|sbin|share|Frameworks))]
|
|
problem "\"#{$1}\" should be \"\#{#{$2.downcase}}\""
|
|
end
|
|
|
|
if line =~ %r[((\#\{prefix\}/share/man/|\#\{man\}/)(man[1-8]))]
|
|
problem "\"#{$1}\" should be \"\#{#{$3}}\""
|
|
end
|
|
|
|
if line =~ %r[((\#\{share\}/(man)))[/'"]]
|
|
problem "\"#{$1}\" should be \"\#{#{$3}}\""
|
|
end
|
|
|
|
if line =~ %r[(\#\{prefix\}/share/(info|man))]
|
|
problem "\"#{$1}\" should be \"\#{#{$2}}\""
|
|
end
|
|
|
|
# Commented-out depends_on
|
|
if line =~ /#\s*depends_on\s+(.+)\s*$/
|
|
problem "Commented-out dep #{$1}"
|
|
end
|
|
|
|
# No trailing whitespace, please
|
|
if line =~ /[\t ]+$/
|
|
problem "#{lineno}: Trailing whitespace was found"
|
|
end
|
|
|
|
if line =~ /if\s+ARGV\.include\?\s+'--(HEAD|devel)'/
|
|
problem "Use \"if build.#{$1.downcase}?\" instead"
|
|
end
|
|
|
|
if line =~ /make && make/
|
|
problem "Use separate make calls"
|
|
end
|
|
|
|
if line =~ /^[ ]*\t/
|
|
problem "Use spaces instead of tabs for indentation"
|
|
end
|
|
|
|
if line =~ /ENV\.x11/
|
|
problem "Use \"depends_on :x11\" instead of \"ENV.x11\""
|
|
end
|
|
|
|
# Avoid hard-coding compilers
|
|
if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?(gcc|llvm-gcc|clang)['" ]}
|
|
problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{$3}\""
|
|
end
|
|
|
|
if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?((g|llvm-g|clang)\+\+)['" ]}
|
|
problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{$3}\""
|
|
end
|
|
|
|
if line =~ /system\s+['"](env|export)(\s+|['"])/
|
|
problem "Use ENV instead of invoking '#{$1}' to modify the environment"
|
|
end
|
|
|
|
if line =~ /version == ['"]HEAD['"]/
|
|
problem "Use 'build.head?' instead of inspecting 'version'"
|
|
end
|
|
|
|
if line =~ /build\.include\?[\s\(]+['"]\-\-(.*)['"]/
|
|
problem "Reference '#{$1}' without dashes"
|
|
end
|
|
|
|
if line =~ /build\.include\?[\s\(]+['"]with(out)?-(.*)['"]/
|
|
problem "Use build.with#{$1}? \"#{$2}\" instead of build.include? 'with#{$1}-#{$2}'"
|
|
end
|
|
|
|
if line =~ /build\.with\?[\s\(]+['"]-?-?with-(.*)['"]/
|
|
problem "Don't duplicate 'with': Use `build.with? \"#{$1}\"` to check for \"--with-#{$1}\""
|
|
end
|
|
|
|
if line =~ /build\.without\?[\s\(]+['"]-?-?without-(.*)['"]/
|
|
problem "Don't duplicate 'without': Use `build.without? \"#{$1}\"` to check for \"--without-#{$1}\""
|
|
end
|
|
|
|
if line =~ /unless build\.with\?(.*)/
|
|
problem "Use if build.without?#{$1} instead of unless build.with?#{$1}"
|
|
end
|
|
|
|
if line =~ /unless build\.without\?(.*)/
|
|
problem "Use if build.with?#{$1} instead of unless build.without?#{$1}"
|
|
end
|
|
|
|
if line =~ /(not\s|!)\s*build\.with?\?/
|
|
problem "Don't negate 'build.without?': use 'build.with?'"
|
|
end
|
|
|
|
if line =~ /(not\s|!)\s*build\.without?\?/
|
|
problem "Don't negate 'build.with?': use 'build.without?'"
|
|
end
|
|
|
|
if line =~ /ARGV\.(?!(debug\?|verbose\?|value[\(\s]))/
|
|
problem "Use build instead of ARGV to check options"
|
|
end
|
|
|
|
if line =~ /def options/
|
|
problem "Use new-style option definitions"
|
|
end
|
|
|
|
if line =~ /def test$/
|
|
problem "Use new-style test definitions (test do)"
|
|
end
|
|
|
|
if line =~ /MACOS_VERSION/
|
|
problem "Use MacOS.version instead of MACOS_VERSION"
|
|
end
|
|
|
|
cats = %w{leopard snow_leopard lion mountain_lion}.join("|")
|
|
if line =~ /MacOS\.(?:#{cats})\?/
|
|
problem "\"#{$&}\" is deprecated, use a comparison to MacOS.version instead"
|
|
end
|
|
|
|
if line =~ /skip_clean\s+:all/
|
|
problem "`skip_clean :all` is deprecated; brew no longer strips symbols\n" +
|
|
"\tPass explicit paths to prevent Homebrew from removing empty folders."
|
|
end
|
|
|
|
if line =~ /depends_on [A-Z][\w:]+\.new$/
|
|
problem "`depends_on` can take requirement classes instead of instances"
|
|
end
|
|
|
|
if line =~ /^def (\w+).*$/
|
|
problem "Define method #{$1.inspect} in the class body, not at the top-level"
|
|
end
|
|
|
|
if line =~ /ENV.fortran/
|
|
problem "Use `depends_on :fortran` instead of `ENV.fortran`"
|
|
end
|
|
|
|
if line =~ /depends_on :(.+) (if.+|unless.+)$/
|
|
audit_conditional_dep($1.to_sym, $2, $&)
|
|
end
|
|
|
|
if line =~ /depends_on ['"](.+)['"] (if.+|unless.+)$/
|
|
audit_conditional_dep($1, $2, $&)
|
|
end
|
|
|
|
if line =~ /(Dir\[("[^\*{},]+")\])/
|
|
problem "#{$1} is unnecessary; just use #{$2}"
|
|
end
|
|
|
|
if line =~ /system (["'](#{FILEUTILS_METHODS})["' ])/o
|
|
system = $1
|
|
method = $2
|
|
problem "Use the `#{method}` Ruby method instead of `system #{system}`"
|
|
end
|
|
|
|
if @strict
|
|
if line =~ /system (["'][^"' ]*(?:\s[^"' ]*)+["'])/
|
|
bad_system = $1
|
|
good_system = bad_system.gsub(" ", "\", \"")
|
|
problem "Use `system #{good_system}` instead of `system #{bad_system}` "
|
|
end
|
|
|
|
if line =~ /(require ["']formula["'])/
|
|
problem "`#{$1}` is now unnecessary"
|
|
end
|
|
end
|
|
end
|
|
|
|
def audit_conditional_dep(dep, condition, line)
|
|
quoted_dep = quote_dep(dep)
|
|
dep = Regexp.escape(dep.to_s)
|
|
|
|
case condition
|
|
when /if build\.include\? ['"]with-#{dep}['"]$/, /if build\.with\? ['"]#{dep}['"]$/
|
|
problem %{Replace #{line.inspect} with "depends_on #{quoted_dep} => :optional"}
|
|
when /unless build\.include\? ['"]without-#{dep}['"]$/, /unless build\.without\? ['"]#{dep}['"]$/
|
|
problem %{Replace #{line.inspect} with "depends_on #{quoted_dep} => :recommended"}
|
|
end
|
|
end
|
|
|
|
def quote_dep(dep)
|
|
Symbol === dep ? dep.inspect : "'#{dep}'"
|
|
end
|
|
|
|
def audit_check_output(output)
|
|
problem(output) if output
|
|
end
|
|
|
|
def audit
|
|
audit_file
|
|
audit_class
|
|
audit_specs
|
|
audit_urls
|
|
audit_deps
|
|
audit_conflicts
|
|
audit_options
|
|
audit_patches
|
|
audit_text
|
|
text.without_patch.split("\n").each_with_index { |line, lineno| audit_line(line, lineno+1) }
|
|
audit_installed
|
|
end
|
|
|
|
private
|
|
|
|
def problem p
|
|
@problems << p
|
|
end
|
|
|
|
def head_only?(formula)
|
|
formula.head && formula.stable.nil?
|
|
end
|
|
end
|
|
|
|
class ResourceAuditor
|
|
attr_reader :problems
|
|
attr_reader :version, :checksum, :using, :specs, :url, :name
|
|
|
|
def initialize(resource)
|
|
@name = resource.name
|
|
@version = resource.version
|
|
@checksum = resource.checksum
|
|
@url = resource.url
|
|
@using = resource.using
|
|
@specs = resource.specs
|
|
@problems = []
|
|
end
|
|
|
|
def audit
|
|
audit_version
|
|
audit_checksum
|
|
audit_download_strategy
|
|
self
|
|
end
|
|
|
|
def audit_version
|
|
if version.nil?
|
|
problem "missing version"
|
|
elsif version.to_s.empty?
|
|
problem "version is set to an empty string"
|
|
elsif not version.detected_from_url?
|
|
version_text = version
|
|
version_url = Version.detect(url, specs)
|
|
if version_url.to_s == version_text.to_s && version.instance_of?(Version)
|
|
problem "version #{version_text} is redundant with version scanned from URL"
|
|
end
|
|
end
|
|
|
|
if version.to_s =~ /^v/
|
|
problem "version #{version} should not have a leading 'v'"
|
|
end
|
|
end
|
|
|
|
def audit_checksum
|
|
return unless checksum
|
|
|
|
case checksum.hash_type
|
|
when :md5
|
|
problem "MD5 checksums are deprecated, please use SHA1 or SHA256"
|
|
return
|
|
when :sha1 then len = 40
|
|
when :sha256 then len = 64
|
|
end
|
|
|
|
if checksum.empty?
|
|
problem "#{checksum.hash_type} is empty"
|
|
else
|
|
problem "#{checksum.hash_type} should be #{len} characters" unless checksum.hexdigest.length == len
|
|
problem "#{checksum.hash_type} contains invalid characters" unless checksum.hexdigest =~ /^[a-fA-F0-9]+$/
|
|
problem "#{checksum.hash_type} should be lowercase" unless checksum.hexdigest == checksum.hexdigest.downcase
|
|
end
|
|
end
|
|
|
|
def audit_download_strategy
|
|
if url =~ %r[^(cvs|bzr|hg|fossil)://] || url =~ %r[^(svn)\+http://]
|
|
problem "Use of the #{$&} scheme is deprecated, pass `:using => :#{$1}` instead"
|
|
end
|
|
|
|
return unless using
|
|
|
|
if using == :ssl3 || using == CurlSSL3DownloadStrategy
|
|
problem "The SSL3 download strategy is deprecated, please choose a different URL"
|
|
elsif using == CurlUnsafeDownloadStrategy || using == UnsafeSubversionDownloadStrategy
|
|
problem "#{using.name} is deprecated, please choose a different URL"
|
|
end
|
|
|
|
if using == :cvs
|
|
mod = specs[:module]
|
|
|
|
if mod == name
|
|
problem "Redundant :module value in URL"
|
|
end
|
|
|
|
if url =~ %r[:[^/]+$]
|
|
mod = url.split(":").last
|
|
|
|
if mod == name
|
|
problem "Redundant CVS module appended to URL"
|
|
else
|
|
problem "Specify CVS module as `:module => \"#{mod}\"` instead of appending it to the URL"
|
|
end
|
|
end
|
|
end
|
|
|
|
url_strategy = DownloadStrategyDetector.detect(url)
|
|
using_strategy = DownloadStrategyDetector.detect('', using)
|
|
|
|
if url_strategy == using_strategy
|
|
problem "Redundant :using value in URL"
|
|
end
|
|
end
|
|
|
|
def problem text
|
|
@problems << text
|
|
end
|
|
end
|