Mike McQuaid 03ace9b110 Require more HTTP mirrors for old OS X versions.
This allows the bootstrap of `curl` and `git` on versions of Mac OS X
that cannot reliably download from HTTPS servers any longer. Once these
are both installed users are able to update Homebrew and download files
securely.

Also, as we're doing this, don't point 10.5 users to Tigerbrew as they
are already given caveats for using Homebrew itself.
2017-09-14 19:58:37 +01:00

1230 lines
41 KiB
Ruby

#: * `audit` [`--strict`] [`--fix`] [`--online`] [`--new-formula`] [`--display-cop-names`] [`--display-filename`] [`--only=`<method>|`--except=`<method>] [`--only-cops=`[COP1,COP2..]|`--except-cops=`[COP1,COP2..]] [<formulae>]:
#: Check <formulae> for Homebrew coding style violations. This should be
#: run before submitting a new formula.
#:
#: If no <formulae> are provided, all of them are checked.
#:
#: If `--strict` is passed, additional checks are run, including RuboCop
#: style checks.
#:
#: If `--fix` is passed, style violations will be
#: automatically fixed using RuboCop's `--auto-correct` feature.
#:
#: If `--online` is passed, additional slower checks that require a network
#: connection are run.
#:
#: If `--new-formula` is passed, various additional checks are run that check
#: if a new formula is eligible for Homebrew. This should be used when creating
#: new formulae and implies `--strict` and `--online`.
#:
#: If `--display-cop-names` is passed, the RuboCop cop name for each violation
#: is included in the output.
#:
#: If `--display-filename` is passed, every line of output is prefixed with the
#: name of the file or formula being audited, to make the output easy to grep.
#:
#: If `--only` is passed, only the methods named `audit_<method>` will be run.
#:
#: If `--except` is passed, the methods named `audit_<method>` will not be run.
#:
#: If `--only-cops` is passed, only the given Rubocop cop(s)' violations would be checked.
#:
#: If `--except-cops` is passed, the given Rubocop cop(s)' checks would be skipped.
#:
#: `audit` exits with a non-zero status if any errors are found. This is useful,
#: for instance, for implementing pre-commit hooks.
# Undocumented options:
# -D activates debugging and profiling of the audit methods (not the same as --debug)
require "formula"
require "formula_versions"
require "utils"
require "extend/ENV"
require "formula_cellar_checks"
require "official_taps"
require "cmd/search"
require "cmd/style"
require "date"
require "missing_formula"
require "digest"
module Homebrew
module_function
def audit
Homebrew.inject_dump_stats!(FormulaAuditor, /^audit_/) if ARGV.switch? "D"
Homebrew.auditing = true
formula_count = 0
problem_count = 0
new_formula = ARGV.include? "--new-formula"
strict = new_formula || ARGV.include?("--strict")
online = new_formula || ARGV.include?("--online")
ENV.activate_extensions!
ENV.setup_build_environment
if ARGV.named.empty?
ff = Formula
files = Tap.map(&:formula_dir)
else
ff = ARGV.resolved_formulae
files = ARGV.resolved_formulae.map(&:path)
end
only_cops = ARGV.value("only-cops").to_s.split(",")
except_cops = ARGV.value("except-cops").to_s.split(",")
if !only_cops.empty? && !except_cops.empty?
odie "--only-cops and --except-cops cannot be used simultaneously!"
elsif (!only_cops.empty? || !except_cops.empty?) && (strict || ARGV.value("only"))
odie "--only-cops/--except-cops and --strict/--only cannot be used simultaneously"
end
options = { fix: ARGV.flag?("--fix"), realpath: true }
if !only_cops.empty?
options[:only_cops] = only_cops
ARGV.push("--only=style")
elsif new_formula
nil
elsif strict
options[:except_cops] = [:NewFormulaAudit]
elsif !except_cops.empty?
options[:except_cops] = except_cops
elsif !strict
options[:only_cops] = [:FormulaAudit]
end
# Check style in a single batch run up front for performance
style_results = check_style_json(files, options)
ff.each do |f|
options = { new_formula: new_formula, strict: strict, online: online }
options[:style_offenses] = style_results.file_offenses(f.path)
fa = FormulaAuditor.new(f, options)
fa.audit
next if fa.problems.empty?
fa.problems
formula_count += 1
problem_count += fa.problems.size
problem_lines = fa.problems.map { |p| "* #{p.chomp.gsub("\n", "\n ")}" }
if ARGV.include? "--display-filename"
puts problem_lines.map { |s| "#{f.path}: #{s}" }
else
puts "#{f.full_name}:", problem_lines.map { |s| " #{s}" }
end
end
return if problem_count.zero?
ofail "#{Formatter.pluralize(problem_count, "problem")} in #{Formatter.pluralize(formula_count, "formula")}"
end
end
class FormulaText
def initialize(path)
@text = path.open("rb", &:read)
@lines = @text.lines.to_a
end
def without_patch
@text.split("\n__END__").first
end
def data?
/^[^#]*\bDATA\b/ =~ @text
end
def end?
/^__END__$/ =~ @text
end
def trailing_newline?
/\Z\n/ =~ @text
end
def =~(other)
other =~ @text
end
def include?(s)
@text.include? s
end
def line_number(regex, skip = 0)
index = @lines.drop(skip).index { |line| line =~ regex }
index ? index + 1 : nil
end
def reverse_line_number(regex)
index = @lines.reverse.index { |line| line =~ regex }
index ? @lines.count - index : nil
end
end
class FormulaAuditor
include FormulaCellarChecks
attr_reader :formula, :text, :problems
BUILD_TIME_DEPS = %w[
autoconf
automake
boost-build
bsdmake
cmake
godep
imake
intltool
libtool
pkg-config
scons
smake
sphinx-doc
swig
].freeze
FILEUTILS_METHODS = FileUtils.singleton_methods(false).map { |m| Regexp.escape(m) }.join "|"
def initialize(formula, options = {})
@formula = formula
@new_formula = options[:new_formula]
@strict = options[:strict]
@online = options[:online]
# Accept precomputed style offense results, for efficiency
@style_offenses = options[:style_offenses]
@problems = []
@text = FormulaText.new(formula.path)
@specs = %w[stable devel head].map { |s| formula.send(s) }.compact
end
def self.check_http_content(url, user_agents: [:default], check_content: false, strict: false, require_http: false)
return unless url.start_with? "http"
details = nil
user_agent = nil
hash_needed = url.start_with?("http:") && !require_http
user_agents.each do |ua|
details = http_content_headers_and_checksum(url, hash_needed: hash_needed, user_agent: ua)
user_agent = ua
break if details[:status].to_s.start_with?("2")
end
return "The URL #{url} is not reachable" unless details[:status]
unless details[:status].start_with? "2"
return "The URL #{url} is not reachable (HTTP status code #{details[:status]})"
end
return unless hash_needed
secure_url = url.sub "http", "https"
secure_details =
http_content_headers_and_checksum(secure_url, hash_needed: true, user_agent: user_agent)
if !details[:status].to_s.start_with?("2") ||
!secure_details[:status].to_s.start_with?("2")
return
end
etag_match = details[:etag] &&
details[:etag] == secure_details[:etag]
content_length_match =
details[:content_length] &&
details[:content_length] == secure_details[:content_length]
file_match = details[:file_hash] == secure_details[:file_hash]
if etag_match || content_length_match || file_match
return "The URL #{url} should use HTTPS rather than HTTP"
end
return unless check_content
no_protocol_file_contents = %r{https?:\\?/\\?/}
details[:file] = details[:file].gsub(no_protocol_file_contents, "/")
secure_details[:file] = secure_details[:file].gsub(no_protocol_file_contents, "/")
# Check for the same content after removing all protocols
if details[:file] == secure_details[:file]
return "The URL #{url} should use HTTPS rather than HTTP"
end
return unless strict
# Same size, different content after normalization
# (typical causes: Generated ID, Timestamp, Unix time)
if details[:file].length == secure_details[:file].length
return "The URL #{url} may be able to use HTTPS rather than HTTP. Please verify it in a browser."
end
lenratio = (100 * secure_details[:file].length / details[:file].length).to_i
return unless (90..110).cover?(lenratio)
"The URL #{url} may be able to use HTTPS rather than HTTP. Please verify it in a browser."
end
def self.http_content_headers_and_checksum(url, hash_needed: false, user_agent: :default)
max_time = hash_needed ? "600" : "25"
output, = curl_output(
"--connect-timeout", "15", "--include", "--max-time", max_time, "--location", url,
user_agent: user_agent
)
status_code = :unknown
while status_code == :unknown || status_code.to_s.start_with?("3")
headers, _, output = output.partition("\r\n\r\n")
status_code = headers[%r{HTTP\/.* (\d+)}, 1]
end
output_hash = Digest::SHA256.digest(output) if hash_needed
{
status: status_code,
etag: headers[%r{ETag: ([wW]\/)?"(([^"]|\\")*)"}, 2],
content_length: headers[/Content-Length: (\d+)/, 1],
file_hash: output_hash,
file: output,
}
end
def audit_style
return unless @style_offenses
display_cop_names = ARGV.include?("--display-cop-names")
@style_offenses.each do |offense|
problem offense.to_s(display_cop_name: display_cop_names)
end
end
def audit_file
# Under normal circumstances (umask 0022), we expect a file mode of 644. If
# the user's umask is more restrictive, respect that by masking out the
# corresponding bits. (The also included 0100000 flag means regular file.)
wanted_mode = 0100644 & ~File.umask
actual_mode = formula.path.stat.mode
unless actual_mode == wanted_mode
problem format("Incorrect file permissions (%03o): chmod %03o %s",
actual_mode & 0777, wanted_mode & 0777, formula.path)
end
problem "'DATA' was found, but no '__END__'" if text.data? && !text.end?
if text.end? && !text.data?
problem "'__END__' was found, but 'DATA' is not used"
end
if text =~ /inreplace [^\n]* do [^\n]*\n[^\n]*\.gsub![^\n]*\n\ *end/m
problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)."
end
problem "File should end with a newline" unless text.trailing_newline?
if formula.versioned_formula?
unversioned_formula = begin
# build this ourselves as we want e.g. homebrew/core to be present
full_name = if formula.tap
"#{formula.tap}/#{formula.name}"
else
formula.name
end
Formulary.factory(full_name.gsub(/@.*$/, "")).path
rescue FormulaUnavailableError, TapFormulaAmbiguityError,
TapFormulaWithOldnameAmbiguityError
Pathname.new formula.path.to_s.gsub(/@.*\.rb$/, ".rb")
end
unless unversioned_formula.exist?
unversioned_name = unversioned_formula.basename(".rb")
problem "#{formula} is versioned but no #{unversioned_name} formula exists"
end
elsif ARGV.build_stable? && formula.stable? &&
!(versioned_formulae = Dir[formula.path.to_s.gsub(/\.rb$/, "@*.rb")]).empty?
versioned_aliases = formula.aliases.grep(/.@\d/)
_, last_alias_version =
File.basename(versioned_formulae.sort.reverse.first)
.gsub(/\.rb$/, "").split("@")
major, minor, = formula.version.to_s.split(".")
alias_name_major = "#{formula.name}@#{major}"
alias_name_major_minor = "#{alias_name_major}.#{minor}"
alias_name = if last_alias_version.split(".").length == 1
alias_name_major
else
alias_name_major_minor
end
valid_alias_names = [alias_name_major, alias_name_major_minor]
if formula.tap && !formula.tap.core_tap?
versioned_aliases.map! { |a| "#{formula.tap}/#{a}" }
valid_alias_names.map! { |a| "#{formula.tap}/#{a}" }
end
valid_versioned_aliases = versioned_aliases & valid_alias_names
invalid_versioned_aliases = versioned_aliases - valid_alias_names
if valid_versioned_aliases.empty?
if formula.tap
problem <<-EOS.undent
Formula has other versions so create a versioned alias:
cd #{formula.tap.alias_dir}
ln -s #{formula.path.to_s.gsub(formula.tap.path, "..")} #{alias_name}
EOS
else
problem "Formula has other versions so create an alias named #{alias_name}."
end
end
unless invalid_versioned_aliases.empty?
problem <<-EOS.undent
Formula has invalid versioned aliases:
#{invalid_versioned_aliases.join("\n ")}
EOS
end
end
end
# core aliases + tap alias names + tap alias full name
@@aliases ||= Formula.aliases + Formula.tap_aliases
def audit_formula_name
return unless @strict
# skip for non-official taps
return if formula.tap.nil? || !formula.tap.official?
name = formula.name
full_name = formula.full_name
if Homebrew::MissingFormula.blacklisted_reason(name)
problem "'#{name}' is blacklisted."
end
if Formula.aliases.include? name
problem "Formula name conflicts with existing aliases."
return
end
if oldname = CoreTap.instance.formula_renames[name]
problem "'#{name}' is reserved as the old name of #{oldname}"
return
end
if !formula.core_formula? && Formula.core_names.include?(name)
problem "Formula name conflicts with existing core formula."
return
end
@@local_official_taps_name_map ||= Tap.select(&:official?).flat_map(&:formula_names)
.each_with_object({}) do |tap_formula_full_name, name_map|
tap_formula_name = tap_formula_full_name.split("/").last
name_map[tap_formula_name] ||= []
name_map[tap_formula_name] << tap_formula_full_name
name_map
end
same_name_tap_formulae = @@local_official_taps_name_map[name] || []
if @online
Homebrew.search_taps(name, silent: true).each do |tap_formula_full_name|
tap_formula_name = tap_formula_full_name.split("/").last
next if tap_formula_name != name
same_name_tap_formulae << tap_formula_full_name
end
end
same_name_tap_formulae.delete(full_name)
return if same_name_tap_formulae.empty?
problem "Formula name conflicts with #{same_name_tap_formulae.join ", "}"
end
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
rescue TapFormulaAmbiguityError
problem "Ambiguous dependency #{dep.name.inspect}."
next
rescue TapFormulaWithOldnameAmbiguityError
problem "Ambiguous oldname dependency #{dep.name.inspect}."
next
end
if dep_f.oldname && dep.name.split("/").last == dep_f.oldname
problem "Dependency '#{dep.name}' was renamed; use new name '#{dep_f.name}'."
end
if @@aliases.include?(dep.name) &&
(dep_f.core_formula? || !dep_f.versioned_formula?)
problem "Dependency '#{dep.name}' is an alias; use the canonical name '#{dep.to_formula.full_name}'."
end
if @new_formula && dep_f.keg_only_reason &&
!["openssl", "apr", "apr-util"].include?(dep.name) &&
[: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."
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 "git"
problem "Don't use git as a dependency"
when "mercurial"
problem "Use `depends_on :hg` instead of `depends_on 'mercurial'`"
when "gfortran"
problem "Use `depends_on :fortran` instead of `depends_on 'gfortran'`"
when "ruby"
problem <<-EOS.undent
Don't use "ruby" as a dependency. If this formula requires a
minimum Ruby version not provided by the system you should
use the RubyRequirement:
depends_on :ruby => "1.8"
where "1.8" is the minimum version of Ruby required.
EOS
when "open-mpi", "mpich"
problem <<-EOS.undent
There are multiple conflicting ways to install MPI. Use an MPIRequirement:
depends_on :mpi => [<lang list>]
Where <lang list> is a comma delimited list that can include:
:cc, :cxx, :f77, :f90
EOS
when *BUILD_TIME_DEPS
next if dep.build? || dep.run?
problem <<-EOS.undent
#{dep} dependency should be
depends_on "#{dep}" => :build
Or if it is indeed a runtime dependency
depends_on "#{dep}" => :run
EOS
end
end
end
end
def audit_conflicts
formula.conflicts.each do |c|
begin
Formulary.factory(c.name)
rescue TapFormulaUnavailableError
# Don't complain about missing cross-tap conflicts.
next
rescue FormulaUnavailableError
problem "Can't find conflicting formula #{c.name.inspect}."
rescue TapFormulaAmbiguityError, TapFormulaWithOldnameAmbiguityError
problem "Ambiguous conflicting formula #{c.name.inspect}."
end
end
end
def audit_keg_only_style
return unless @strict
return unless formula.keg_only?
whitelist = %w[
Apple
macOS
OS
Homebrew
Xcode
GPG
GNOME
BSD
Firefox
].freeze
reason = formula.keg_only_reason.to_s
# Formulae names can legitimately be uppercase/lowercase/both.
name = Regexp.new(formula.name, Regexp::IGNORECASE)
reason.sub!(name, "")
first_word = reason.split[0]
if reason =~ /\A[A-Z]/ && !reason.start_with?(*whitelist)
problem <<-EOS.undent
'#{first_word}' from the keg_only reason should be '#{first_word.downcase}'.
EOS
end
return unless reason.end_with?(".")
problem "keg_only reason should not end with a period."
end
def audit_homepage
homepage = formula.homepage
return if homepage.nil? || homepage.empty?
return unless @online
return unless DevelopmentTools.curl_handles_most_https_homepages?
if http_content_problem = FormulaAuditor.check_http_content(homepage,
user_agents: [:browser, :default],
check_content: true,
strict: @strict)
problem http_content_problem
end
end
def audit_bottle_spec
return unless formula.bottle_disabled?
return if formula.bottle_disable_reason.valid?
problem "Unrecognized bottle modifier"
end
def audit_github_repository
return unless @online
return unless @new_formula
regex = %r{https?://github\.com/([^/]+)/([^/]+)/?.*}
_, user, repo = *regex.match(formula.stable.url) if formula.stable
_, user, repo = *regex.match(formula.homepage) unless user
return if !user || !repo
repo.gsub!(/.git$/, "")
begin
metadata = GitHub.repository(user, repo)
rescue GitHub::HTTPNotFoundError
return
end
return if metadata.nil?
problem "GitHub fork (not canonical repository)" if metadata["fork"]
if (metadata["forks_count"] < 20) && (metadata["subscribers_count"] < 20) &&
(metadata["stargazers_count"] < 50)
problem "GitHub repository not notable enough (<20 forks, <20 watchers and <50 stars)"
end
return if Date.parse(metadata["created_at"]) <= (Date.today - 30)
problem "GitHub repository too new (<30 days old)"
end
def audit_specs
if head_only?(formula) && formula.tap.to_s.downcase !~ %r{[-/]head-only$}
problem "Head-only (no stable download)"
end
if devel_only?(formula) && formula.tap.to_s.downcase !~ %r{[-/]devel-only$}
problem "Devel-only (no stable download)"
end
%w[Stable Devel HEAD].each do |name|
spec_name = name.downcase.to_sym
next unless spec = formula.send(spec_name)
ra = ResourceAuditor.new(spec, spec_name, online: @online, strict: @strict).audit
problems.concat ra.problems.map { |problem| "#{name}: #{problem}" }
spec.resources.each_value do |resource|
ra = ResourceAuditor.new(resource, spec_name, online: @online, strict: @strict).audit
problems.concat ra.problems.map { |problem|
"#{name} resource #{resource.name.inspect}: #{problem}"
}
end
next if spec.patches.empty?
next unless @new_formula
problem "New formulae should not require patches to build. Patches should be submitted and accepted upstream first."
end
%w[Stable Devel].each do |name|
next unless spec = formula.send(name.downcase)
version = spec.version
if version.to_s !~ /\d/
problem "#{name}: version (#{version}) is set to a string without a digit"
end
if version.to_s.start_with?("HEAD")
problem "#{name}: non-HEAD version name (#{version}) should not begin with HEAD"
end
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
unstable_whitelist = %w[
aalib 1.4rc5
angolmois 2.0.0alpha2
automysqlbackup 3.0-rc6
aview 1.3.0rc1
distcc 3.2rc1
elm-format 0.6.0-alpha
ftgl 2.1.3-rc5
hidapi 0.8.0-rc1
libcaca 0.99b19
nethack4 4.3.0-beta2
opensyobon 1.0rc2
premake 4.4-beta5
pwnat 0.3-beta
pxz 4.999.9
recode 3.7-beta2
speexdsp 1.2rc3
sqoop 1.4.6
tcptraceroute 1.5beta7
testssl 2.8rc3
tiny-fugue 5.0b8
vbindiff 3.0_beta4
].each_slice(2).to_a.map do |formula, version|
[formula, version.sub(/\d+$/, "")]
end
gnome_devel_whitelist = %w[
gtk-doc 1.25
libart 2.3.21
pygtkglext 1.1.0
].each_slice(2).to_a.map do |formula, version|
[formula, version.split(".")[0..1].join(".")]
end
stable = formula.stable
case stable && stable.url
when /[\d\._-](alpha|beta|rc\d)/
matched = Regexp.last_match(1)
version_prefix = stable.version.to_s.sub(/\d+$/, "")
return if unstable_whitelist.include?([formula.name, version_prefix])
problem "Stable version URLs should not contain #{matched}"
when %r{download\.gnome\.org/sources}, %r{ftp\.gnome\.org/pub/GNOME/sources}i
version_prefix = stable.version.to_s.split(".")[0..1].join(".")
return if gnome_devel_whitelist.include?([formula.name, version_prefix])
version = Version.parse(stable.url)
if version >= Version.create("1.0")
minor_version = version.to_s.split(".", 3)[1].to_i
if minor_version.odd?
problem "#{stable.version} is a development release"
end
end
end
end
def audit_revision_and_version_scheme
return unless formula.tap # skip formula not from core or any taps
return unless formula.tap.git? # git log is required
return if @new_formula
fv = FormulaVersions.new(formula)
previous_version_and_checksum = fv.previous_version_and_checksum("origin/master")
[:stable, :devel].each do |spec_sym|
next unless spec = formula.send(spec_sym)
next unless previous_version_and_checksum[spec_sym][:version] == spec.version
next if previous_version_and_checksum[spec_sym][:checksum] == spec.checksum
problem "#{spec_sym}: sha256 changed without the version also changing; please create an issue upstream to rule out malicious circumstances and to find out why the file changed."
end
attributes = [:revision, :version_scheme]
attributes_map = fv.version_attributes_map(attributes, "origin/master")
current_version_scheme = formula.version_scheme
[:stable, :devel].each do |spec|
spec_version_scheme_map = attributes_map[:version_scheme][spec]
next if spec_version_scheme_map.empty?
version_schemes = spec_version_scheme_map.values.flatten
max_version_scheme = version_schemes.max
max_version = spec_version_scheme_map.select do |_, version_scheme|
version_scheme.first == max_version_scheme
end.keys.max
if max_version_scheme && current_version_scheme < max_version_scheme
problem "version_scheme should not decrease (from #{max_version_scheme} to #{current_version_scheme})"
end
if max_version_scheme && current_version_scheme >= max_version_scheme &&
current_version_scheme > 1 &&
!version_schemes.include?(current_version_scheme - 1)
problem "version_schemes should only increment by 1"
end
formula_spec = formula.send(spec)
next unless formula_spec
spec_version = formula_spec.version
next unless max_version
next if spec_version >= max_version
above_max_version_scheme = current_version_scheme > max_version_scheme
map_includes_version = spec_version_scheme_map.keys.include?(spec_version)
next if !current_version_scheme.zero? &&
(above_max_version_scheme || map_includes_version)
problem "#{spec} version should not decrease (from #{max_version} to #{spec_version})"
end
current_revision = formula.revision
revision_map = attributes_map[:revision][:stable]
if formula.stable && !revision_map.empty?
stable_revisions = revision_map[formula.stable.version]
stable_revisions ||= []
max_revision = stable_revisions.max || 0
if current_revision < max_revision
problem "revision should not decrease (from #{max_revision} to #{current_revision})"
end
stable_revisions -= [formula.revision]
if !current_revision.zero? && stable_revisions.empty? &&
revision_map.keys.length > 1
problem "'revision #{formula.revision}' should be removed"
elsif current_revision > 1 &&
current_revision != max_revision &&
!stable_revisions.include?(current_revision - 1)
problem "revisions should only increment by 1"
end
elsif !current_revision.zero? # head/devel-only formula
problem "'revision #{current_revision}' should be removed"
end
end
def audit_text
bin_names = Set.new
bin_names << formula.name
bin_names += formula.aliases
[formula.bin, formula.sbin].each do |dir|
next unless dir.exist?
bin_names += dir.children.map(&:basename).map(&:to_s)
end
bin_names.each do |name|
["system", "shell_output", "pipe_output"].each do |cmd|
if text =~ %r{(def test|test do).*(#{Regexp.escape(HOMEBREW_PREFIX)}/bin/)?#{cmd}[\(\s]+['"]#{Regexp.escape(name)}[\s'"]}m
problem %Q(fully scope test #{cmd} calls e.g. #{cmd} "\#{bin}/#{name}")
end
end
end
end
def audit_lines
text.without_patch.split("\n").each_with_index do |line, lineno|
line_problems(line, lineno + 1)
end
end
def line_problems(line, _lineno)
# Check for string interpolation of single values.
if line =~ /(system|inreplace|gsub!|change_make_var!).*[ ,]"#\{([\w.]+)\}"/
problem "Don't need to interpolate \"#{Regexp.last_match(2)}\" with #{Regexp.last_match(1)}"
end
# Check for string concatenation; prefer interpolation
if line =~ /(#\{\w+\s*\+\s*['"][^}]+\})/
problem "Try not to concatenate paths in string interpolation:\n #{Regexp.last_match(1)}"
end
# Prefer formula path shortcuts in Pathname+
if line =~ %r{\(\s*(prefix\s*\+\s*(['"])(bin|include|libexec|lib|sbin|share|Frameworks)[/'"])}
problem "\"(#{Regexp.last_match(1)}...#{Regexp.last_match(2)})\" should be \"(#{Regexp.last_match(3).downcase}+...)\""
end
if line =~ /((man)\s*\+\s*(['"])(man[1-8])(['"]))/
problem "\"#{Regexp.last_match(1)}\" should be \"#{Regexp.last_match(4)}\""
end
# Prefer formula path shortcuts in strings
if line =~ %r[(\#\{prefix\}/(bin|include|libexec|lib|sbin|share|Frameworks))]
problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(2).downcase}}\""
end
if line =~ %r[((\#\{prefix\}/share/man/|\#\{man\}/)(man[1-8]))]
problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(3)}}\""
end
if line =~ %r[((\#\{share\}/(man)))[/'"]]
problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(3)}}\""
end
if line =~ %r[(\#\{prefix\}/share/(info|man))]
problem "\"#{Regexp.last_match(1)}\" should be \"\#{#{Regexp.last_match(2)}}\""
end
if line =~ /depends_on\s+['"](.+)['"]\s+=>\s+:(lua|perl|python|ruby)(\d*)/
problem "#{Regexp.last_match(2)} modules should be vendored rather than use deprecated `depends_on \"#{Regexp.last_match(1)}\" => :#{Regexp.last_match(2)}#{Regexp.last_match(3)}`"
end
if line =~ /depends_on\s+['"](.+)['"]\s+=>\s+(.*)/
dep = Regexp.last_match(1)
Regexp.last_match(2).split(" ").map do |o|
break if ["if", "unless"].include?(o)
next unless o =~ /^\[?['"](.*)['"]/
problem "Dependency #{dep} should not use option #{Regexp.last_match(1)}"
end
end
if line =~ /if\s+ARGV\.include\?\s+'--(HEAD|devel)'/
problem "Use \"if build.#{Regexp.last_match(1).downcase}?\" instead"
end
problem "Use separate make calls" if line.include?("make && make")
problem "Use spaces instead of tabs for indentation" if line =~ /^[ ]*\t/
if line.include?("ENV.java_cache")
problem "In-formula ENV.java_cache usage has been deprecated & should be removed."
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 \"#{Regexp.last_match(3)}\""
end
if line =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?((g|llvm-g|clang)\+\+)['" ]}
problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{Regexp.last_match(3)}\""
end
if line =~ /system\s+['"](env|export)(\s+|['"])/
problem "Use ENV instead of invoking '#{Regexp.last_match(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 '#{Regexp.last_match(1)}' without dashes"
end
if line =~ /build\.include\?[\s\(]+['"]with(out)?-(.*)['"]/
problem "Use build.with#{Regexp.last_match(1)}? \"#{Regexp.last_match(2)}\" instead of build.include? 'with#{Regexp.last_match(1)}-#{Regexp.last_match(2)}'"
end
if line =~ /build\.with\?[\s\(]+['"]-?-?with-(.*)['"]/
problem "Don't duplicate 'with': Use `build.with? \"#{Regexp.last_match(1)}\"` to check for \"--with-#{Regexp.last_match(1)}\""
end
if line =~ /build\.without\?[\s\(]+['"]-?-?without-(.*)['"]/
problem "Don't duplicate 'without': Use `build.without? \"#{Regexp.last_match(1)}\"` to check for \"--without-#{Regexp.last_match(1)}\""
end
if line =~ /unless build\.with\?(.*)/
problem "Use if build.without?#{Regexp.last_match(1)} instead of unless build.with?#{Regexp.last_match(1)}"
end
if line =~ /unless build\.without\?(.*)/
problem "Use if build.with?#{Regexp.last_match(1)} instead of unless build.without?#{Regexp.last_match(1)}"
end
if line =~ /(not\s|!)\s*build\.with?\?/
problem "Don't negate 'build.with?': use 'build.without?'"
end
if line =~ /(not\s|!)\s*build\.without?\?/
problem "Don't negate 'build.without?': use 'build.with?'"
end
if line =~ /ARGV\.(?!(debug\?|verbose\?|value[\(\s]))/
problem "Use build instead of ARGV to check options"
end
if line.include?("MACOS_VERSION")
problem "Use MacOS.version instead of MACOS_VERSION"
end
if line.include?("MACOS_FULL_VERSION")
problem "Use MacOS.full_version instead of MACOS_FULL_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 =~ /depends_on [A-Z][\w:]+\.new$/
problem "`depends_on` can take requirement classes instead of instances"
end
if line =~ /^def (\w+).*$/
problem "Define method #{Regexp.last_match(1).inspect} in the class body, not at the top-level"
end
if line.include?("ENV.fortran") && !formula.requirements.map(&:class).include?(FortranRequirement)
problem "Use `depends_on :fortran` instead of `ENV.fortran`"
end
if line =~ /JAVA_HOME/i && !formula.requirements.map(&:class).include?(JavaRequirement)
problem "Use `depends_on :java` to set JAVA_HOME"
end
if line =~ /depends_on :(.+) (if.+|unless.+)$/
conditional_dep_problems(Regexp.last_match(1).to_sym, Regexp.last_match(2), $&)
end
if line =~ /depends_on ['"](.+)['"] (if.+|unless.+)$/
conditional_dep_problems(Regexp.last_match(1), Regexp.last_match(2), $&)
end
if line =~ /(Dir\[("[^\*{},]+")\])/
problem "#{Regexp.last_match(1)} is unnecessary; just use #{Regexp.last_match(2)}"
end
if line =~ /system (["'](#{FILEUTILS_METHODS})["' ])/o
system = Regexp.last_match(1)
method = Regexp.last_match(2)
problem "Use the `#{method}` Ruby method instead of `system #{system}`"
end
if line =~ /assert [^!]+\.include?/
problem "Use `assert_match` instead of `assert ...include?`"
end
return unless @strict
problem "`#{Regexp.last_match(1)}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/
if line =~ /system ((["'])[^"' ]*(?:\s[^"' ]*)+\2)/
bad_system = Regexp.last_match(1)
unless %w[| < > & ; *].any? { |c| bad_system.include? c }
good_system = bad_system.gsub(" ", "\", \"")
problem "Use `system #{good_system}` instead of `system #{bad_system}` "
end
end
problem "`#{Regexp.last_match(1)}` is now unnecessary" if line =~ /(require ["']formula["'])/
if line =~ %r{#\{share\}/#{Regexp.escape(formula.name)}[/'"]}
problem "Use \#{pkgshare} instead of \#{share}/#{formula.name}"
end
return unless line =~ %r{share(\s*[/+]\s*)(['"])#{Regexp.escape(formula.name)}(?:\2|/)}
problem "Use pkgshare instead of (share#{Regexp.last_match(1)}\"#{formula.name}\")"
end
def audit_reverse_migration
# Only enforce for new formula being re-added to core and official taps
return unless @strict
return unless formula.tap && formula.tap.official?
return unless formula.tap.tap_migrations.key?(formula.name)
problem <<-EOS.undent
#{formula.name} seems to be listed in tap_migrations.json!
Please remove #{formula.name} from present tap & tap_migrations.json
before submitting it to Homebrew/homebrew-#{formula.tap.repo}.
EOS
end
def audit_prefix_has_contents
return unless formula.prefix.directory?
return unless Keg.new(formula.prefix).empty_installation?
problem <<-EOS.undent
The installation seems to be empty. Please ensure the prefix
is set correctly and expected files are installed.
The prefix configure/make argument may be case-sensitive.
EOS
end
def conditional_dep_problems(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 %Q(Replace #{line.inspect} with "depends_on #{quoted_dep} => :optional")
when /unless build\.include\? ['"]without-#{dep}['"]$/, /unless build\.without\? ['"]#{dep}['"]$/
problem %Q(Replace #{line.inspect} with "depends_on #{quoted_dep} => :recommended")
end
end
def quote_dep(dep)
dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'"
end
def problem_if_output(output)
problem(output) if output
end
def audit
only_audits = ARGV.value("only").to_s.split(",")
except_audits = ARGV.value("except").to_s.split(",")
if !only_audits.empty? && !except_audits.empty?
odie "--only and --except cannot be used simultaneously!"
end
methods.map(&:to_s).grep(/^audit_/).each do |audit_method_name|
name = audit_method_name.gsub(/^audit_/, "")
if !only_audits.empty?
next unless only_audits.include?(name)
elsif !except_audits.empty?
next if except_audits.include?(name)
end
send(audit_method_name)
end
end
private
def problem(p)
@problems << p
end
def head_only?(formula)
formula.head && formula.devel.nil? && formula.stable.nil?
end
def devel_only?(formula)
formula.devel && formula.stable.nil?
end
end
class ResourceAuditor
attr_reader :name, :version, :checksum, :url, :mirrors, :using, :specs, :owner
attr_reader :spec_name, :problems
def initialize(resource, spec_name, options = {})
@name = resource.name
@version = resource.version
@checksum = resource.checksum
@url = resource.url
@mirrors = resource.mirrors
@using = resource.using
@specs = resource.specs
@owner = resource.owner
@spec_name = spec_name
@online = options[:online]
@strict = options[:strict]
@problems = []
end
def audit
audit_version
audit_download_strategy
audit_urls
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 !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.start_with?("v")
problem "version #{version} should not have a leading 'v'"
end
return unless version.to_s =~ /_\d+$/
problem "version #{version} should not end with an underline and a number"
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 => :#{Regexp.last_match(1)}` instead"
end
url_strategy = DownloadStrategyDetector.detect(url)
if using == :git || url_strategy == GitDownloadStrategy
if specs[:tag] && !specs[:revision]
problem "Git should specify :revision when a :tag is specified."
end
end
return unless using
if using == :ssl3 || \
(Object.const_defined?("CurlSSL3DownloadStrategy") && using == CurlSSL3DownloadStrategy)
problem "The SSL3 download strategy is deprecated, please choose a different URL"
elsif (Object.const_defined?("CurlUnsafeDownloadStrategy") && using == CurlUnsafeDownloadStrategy) || \
(Object.const_defined?("UnsafeSubversionDownloadStrategy") && using == UnsafeSubversionDownloadStrategy)
problem "#{using.name} is deprecated, please choose a different URL"
end
if using == :cvs
mod = specs[:module]
problem "Redundant :module value in URL" if mod == name
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
return unless url_strategy == DownloadStrategyDetector.detect("", using)
problem "Redundant :using value in URL"
end
def self.curl_git_openssl_and_deps
@curl_git_openssl_and_deps ||= begin
formulae_names = ["curl", "git", "openssl"]
formulae_names += formulae_names.flat_map do |f|
Formula[f].recursive_dependencies.map(&:name)
end
formulae_names.uniq
rescue FormulaUnavailableError
[]
end
end
def audit_urls
urls = [url] + mirrors
require_http = ResourceAuditor.curl_git_openssl_and_deps.include?(owner.name)
if spec_name == :stable && require_http &&
!urls.find { |u| u.start_with?("http://") }
problem "should always include at least one HTTP mirror"
end
return unless @online
urls.each do |url|
next if !@strict && mirrors.include?(url)
strategy = DownloadStrategyDetector.detect(url, using)
if strategy <= CurlDownloadStrategy && !url.start_with?("file")
# A `brew mirror`'ed URL is usually not yet reachable at the time of
# pull request.
next if url =~ %r{^https://dl.bintray.com/homebrew/mirror/}
if http_content_problem = FormulaAuditor.check_http_content(url, name, require_http: require_http)
problem http_content_problem
end
elsif strategy <= GitDownloadStrategy
unless Utils.git_remote_exists url
problem "The URL #{url} is not a valid git URL"
end
elsif strategy <= SubversionDownloadStrategy
next unless DevelopmentTools.subversion_handles_most_https_certificates?
next unless Utils.svn_available?
unless Utils.svn_remote_exists url
problem "The URL #{url} is not a valid svn URL"
end
end
end
end
def problem(text)
@problems << text
end
end