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

Merge pull request #927 from MikeMcQuaid/dev-cmd-rubocop

Browse Source 
Fix Library/Homebrew/dev-cmd RuboCop warnings
This commit is contained in:
Mike McQuaid 2016-09-12 08:09:29 +01:00 committed by GitHub
commit 59116d08ca
10 changed files with 120 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Library/Homebrew/dev-cmd/aspell-dictionaries.rb
View File

@ -14,8 +14,8 @@ module Homebrew
open("#{dict_url}/0index.html") do |content|
content.each_line do |line|
break if %r{^</table} === line
next unless /^<tr><td><a/ === line
break if %r{^</table} =~ line
next unless /^<tr><td><a/ =~ line
fields = line.split('"')
lang = fields[1]

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

@ -101,20 +101,20 @@ class FormulaText
@text.split("\n__END__").first
end
def has_DATA?
def data?
/^[^#]*\bDATA\b/ =~ @text
end
def has_END?
def end?
/^__END__$/ =~ @text
end
def has_trailing_newline?
def trailing_newline?
/\Z\n/ =~ @text
end
def =~(regex)
regex =~ @text
def =~(other)
other =~ @text
end
def include?(s)
@ -152,15 +152,15 @@ class FormulaAuditor
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]
@new_formula = options[:new_formula]
@strict = options[:strict]
@online = options[:online]
# Accept precomputed style offense results, for efficiency
@style_offenses = options[:style_offenses]
@problems = []
@ -214,7 +214,7 @@ class FormulaAuditor
previous_before = previous_pair[0]
previous_after = previous_pair[1]
end
offset = (previous_after && previous_after[0] && previous_after[0] >= 1) ? previous_after[0] - 1 : 0
offset = previous_after && previous_after[0] && previous_after[0] >= 1 ? previous_after[0] - 1 : 0
present = component_list.map do |regex, name|
lineno = if reverse
text.reverse_line_number regex
@ -229,7 +229,7 @@ class FormulaAuditor
if reverse
# scan in the forward direction from the offset
audit_components(false, [c1, c2]) if c1[0] > c2[0] # at least one more offense
elsif c1[0] > c2[0] && (offset == 0 || previous_pair.nil? || (c1[0] + offset) != previous_before[0] || (c2[0] + offset) != previous_after[0])
elsif c1[0] > c2[0] && (offset.zero? || previous_pair.nil? || (c1[0] + offset) != previous_before[0] || (c2[0] + offset) != previous_after[0])
component_problem c1, c2, offset
no_problem = false
end
@ -251,11 +251,11 @@ class FormulaAuditor
actual_mode & 0777, wanted_mode & 0777, formula.path)
end
if text.has_DATA? && !text.has_END?
if text.data? && !text.end?
problem "'DATA' was found, but no '__END__'"
end
if text.has_END? && !text.has_DATA?
if text.end? && !text.data?
problem "'__END__' was found, but 'DATA' is not used"
end
@ -263,7 +263,7 @@ class FormulaAuditor
problem "'inreplace ... do' was used for a single substitution (use the non-block form instead)."
end
unless text.has_trailing_newline?
unless text.trailing_newline?
problem "File should end with a newline"
end
@ -328,13 +328,13 @@ class FormulaAuditor
return
end
@@local_official_taps_name_map ||= Tap.select(&:official?).flat_map(&:formula_names).
reduce(Hash.new) do |name_map, tap_formula_full_name|
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
@@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] || []
@ -396,14 +396,6 @@ class FormulaAuditor
end
case dep.name
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
when "git"
problem "Don't use git as a dependency"
when "mercurial"
@ -419,6 +411,9 @@ class FormulaAuditor
where "1.8" is the minimum version of Ruby required.
EOS
when "open-mpi", "mpich"
problem <<-EOS.undent
when *BUILD_TIME_DEPS
next if dep.build? || dep.run?
problem <<-EOS.undent
There are multiple conflicting ways to install MPI. Use an MPIRequirement:
depends_on :mpi => [<lang list>]
@ -452,10 +447,9 @@ class FormulaAuditor
problem "Options should begin with with/without. Migrate '--#{o.name}' with `deprecated_option`."
end
if o.name =~ /^with(out)?-(?:checks?|tests)$/
unless formula.deps.any? { |d| d.name == "check" && (d.optional? || d.recommended?) }
problem "Use '--with#{$1}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`."
end
next unless o.name =~ /^with(out)?-(?:checks?|tests)$/
unless formula.deps.any? { |d| d.name == "check" && (d.optional? || d.recommended?) }
problem "Use '--with#{$1}-test' instead of '--#{o.name}'. Migrate '--#{o.name}' with `deprecated_option`."
end
end
end
@ -667,15 +661,14 @@ class FormulaAuditor
attributes.each do |attribute|
attributes_for_version = attributes_map[attribute][formula.version]
if !attributes_for_version.empty?
if formula.send(attribute) < attributes_for_version.max
problem "#{attribute} should not decrease"
end
next if attributes_for_version.empty?
if formula.send(attribute) < attributes_for_version.max
problem "#{attribute} should not decrease"
end
end
revision_map = attributes_map[:revision]
if formula.revision != 0
if formula.revision.nonzero?
if formula.stable
if revision_map[formula.stable.version].empty? # check stable spec
problem "'revision #{formula.revision}' should be removed"
@ -772,7 +765,7 @@ class FormulaAuditor
# FileUtils is included in Formula
# encfs modifies a file with this name, so check for some leading characters
if line =~ /[^'"\/]FileUtils\.(\w+)/
if line =~ %r{[^'"/]FileUtils\.(\w+)}
problem "Don't need 'FileUtils.' before #{$1}."
end
@ -1038,7 +1031,7 @@ class FormulaAuditor
end
def quote_dep(dep)
Symbol === dep ? dep.inspect : "'#{dep}'"
dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'"
end
def audit_check_output(output)

47
Library/Homebrew/dev-cmd/bottle.rb
View File

@ -13,7 +13,7 @@ require "utils/inreplace"
require "erb"
require "extend/pathname"
BOTTLE_ERB = <<-EOS
BOTTLE_ERB = <<-EOS.freeze
bottle do
<% if !root_url.start_with?(BottleSpecification::DEFAULT_DOMAIN) %>
root_url "<%= root_url %>"
@ -89,15 +89,14 @@ module Homebrew
end
end
if ARGV.verbose? && !text_matches.empty?
print_filename string, file
text_matches.first(MAXIMUM_STRING_MATCHES).each do |match, offset|
puts " #{Tty.gray}-->#{Tty.reset} match '#{match}' at offset #{Tty.em}0x#{offset}#{Tty.reset}"
end
next unless ARGV.verbose? && !text_matches.empty?
print_filename string, file
text_matches.first(MAXIMUM_STRING_MATCHES).each do |match, offset|
puts " #{Tty.gray}-->#{Tty.reset} match '#{match}' at offset #{Tty.em}0x#{offset}#{Tty.reset}"
end
if text_matches.size > MAXIMUM_STRING_MATCHES
puts "Only the first #{MAXIMUM_STRING_MATCHES} matches were output"
end
if text_matches.size > MAXIMUM_STRING_MATCHES
puts "Only the first #{MAXIMUM_STRING_MATCHES} matches were output"
end
end
@ -107,10 +106,9 @@ module Homebrew
def keg_contain_absolute_symlink_starting_with?(string, keg)
absolute_symlinks_start_with_string = []
keg.find do |pn|
if pn.symlink? && (link = pn.readlink).absolute?
if link.to_s.start_with?(string)
absolute_symlinks_start_with_string << pn
end
next unless pn.symlink? && (link = pn.readlink).absolute?
if link.to_s.start_with?(string)
absolute_symlinks_start_with_string << pn
end
end
@ -146,7 +144,7 @@ module Homebrew
return
end
unless Utils::Bottles::built_as? f
unless Utils::Bottles.built_as? f
return ofail "Formula not installed with '--build-bottle': #{f.full_name}"
end
@ -289,6 +287,9 @@ module Homebrew
bottle.sha256 sha256 => Utils::Bottles.tag
old_spec = f.bottle_specification
p root_url
p old_spec.root_url(root_url)
p bottle.root_url(root_url)
if ARGV.include?("--keep-old") && !old_spec.checksums.empty?
mismatches = [:root_url, :prefix, :cellar, :rebuild].select do |key|
old_spec.send(key) != bottle.send(key)
@ -332,7 +333,7 @@ module Homebrew
"filename" => filename.to_s,
"sha256" => sha256,
},
}
},
},
"bintray" => {
"package" => Utils::Bottles::Bintray.package(f.name),
@ -372,7 +373,7 @@ module Homebrew
output = bottle_output bottle
if write
path = Pathname.new("#{HOMEBREW_REPOSITORY/bottle_hash["formula"]["path"]}")
path = Pathname.new((HOMEBREW_REPOSITORY/bottle_hash["formula"]["path"]).to_s)
update_or_add = nil
Utils::Inreplace.inreplace(path) do |s|
@ -391,7 +392,7 @@ module Homebrew
old_value = old_value_original.to_s.delete ":'\""
tag = tag.to_s.delete ":"
if !tag.empty?
unless tag.empty?
if !bottle_hash["bottle"]["tags"][tag].to_s.empty?
mismatches << "#{key} => #{tag}"
else
@ -403,11 +404,10 @@ module Homebrew
value_original = bottle_hash["bottle"][key]
value = value_original.to_s
next if key == "cellar" && old_value == "any" && value == "any_skip_relocation"
if old_value.empty? || value != old_value
old_value = old_value_original.inspect
value = value_original.inspect
mismatches << "#{key}: old: #{old_value}, new: #{value}"
end
next unless old_value.empty? || value != old_value
old_value = old_value_original.inspect
value = value_original.inspect
mismatches << "#{key}: old: #{old_value}, new: #{value}"
end
unless mismatches.empty?
@ -444,7 +444,8 @@ module Homebrew
rebuild\ \d+ # rebuild with a number
)\n+ # multiple empty lines
)+
/mx, '\0' + output + "\n")
/mx, '\0' + output + "\n"
)
end
odie "Bottle block addition failed!" unless string
end

2
Library/Homebrew/dev-cmd/bump-formula-pr.rb
View File

@ -101,7 +101,7 @@ module Homebrew
old_formula_version = formula_version(formula, requested_spec)
replacement_pairs = []
if requested_spec == :stable && formula.revision != 0
if requested_spec == :stable && formula.revision.nonzero?
replacement_pairs << [/^ revision \d+\n(\n( head "))?/m, "\\2"]
end

2
Library/Homebrew/dev-cmd/create.rb
View File

@ -91,7 +91,7 @@ module Homebrew
def __gets
gots = $stdin.gets.chomp
if gots.empty? then nil else gots end
gots.empty? ? nil : gots
end
end

4
Library/Homebrew/dev-cmd/edit.rb
View File

@ -44,7 +44,9 @@ module Homebrew
def library_folders
Dir["#{HOMEBREW_LIBRARY}/*"].reject do |d|
case File.basename(d) when "LinkedKegs", "Aliases" then true end
case File.basename(d)
when "LinkedKegs", "Aliases" then true
end
end
end
end

26
Library/Homebrew/dev-cmd/man.rb
View File

@ -34,15 +34,15 @@ module Homebrew
end
def path_glob_commands(glob)
Pathname.glob(glob).
sort_by { |source_file| sort_key_for_path(source_file) }.
map { |source_file|
source_file.read.lines.
grep(/^#:/).
map { |line| line.slice(2..-1) }.
join
}.
reject { |s| s.strip.empty? || s.include?("@hide_from_man_page") }
Pathname.glob(glob)
.sort_by { |source_file| sort_key_for_path(source_file) }
.map do |source_file|
source_file.read.lines
.grep(/^#:/)
.map { |line| line.slice(2..-1) }
.join
end
.reject { |s| s.strip.empty? || s.include?("@hide_from_man_page") }
end
def build_man_page
@ -51,11 +51,11 @@ module Homebrew
variables[:commands] = path_glob_commands("#{HOMEBREW_LIBRARY_PATH}/cmd/*.{rb,sh}")
variables[:developer_commands] = path_glob_commands("#{HOMEBREW_LIBRARY_PATH}/dev-cmd/*.{rb,sh}")
variables[:maintainers] = (HOMEBREW_REPOSITORY/"README.md").
read[/Homebrew's current maintainers are (.*)\./, 1].
scan(/\[([^\]]*)\]/).flatten
variables[:maintainers] = (HOMEBREW_REPOSITORY/"README.md")
.read[/Homebrew's current maintainers are (.*)\./, 1]
.scan(/\[([^\]]*)\]/).flatten
ERB.new(template, nil, ">").result(variables.instance_eval{ binding })
ERB.new(template, nil, ">").result(variables.instance_eval { binding })
end
def sort_key_for_path(path)

17
Library/Homebrew/dev-cmd/pull.rb
View File

@ -237,7 +237,7 @@ module Homebrew
private
def publish_changed_formula_bottles(tap, changed_formulae_names)
def publish_changed_formula_bottles(_tap, changed_formulae_names)
if ENV["HOMEBREW_DISABLE_LOAD_FORMULA"]
raise "Need to load formulae to publish them!"
end
@ -360,7 +360,13 @@ module Homebrew
def subject_for_bump(formula, old, new)
if old[:nonexistent]
# New formula
headline_ver = new[:stable] ? new[:stable] : new[:devel] ? new[:devel] : new[:head]
headline_ver = if new[:stable]
new[:stable]
elsif new[:devel]
new[:devel]
else
new[:head]
end
subject = "#{formula.name} #{headline_ver} (new formula)"
else
# Update to existing formula
@ -430,7 +436,7 @@ module Homebrew
FormulaInfoFromJson.new(Utils::JSON.load(json)[0])
end
def bottle_tags()
def bottle_tags
return [] unless info["bottle"]["stable"]
info["bottle"]["stable"]["files"].keys
end
@ -467,7 +473,6 @@ module Homebrew
end
end
# Bottle info as used internally by pull, with alternate platform support
class BottleInfo
# URL of bottle as string
@ -522,7 +527,7 @@ module Homebrew
http.use_ssl = true
retry_count = 0
http.start do
while true do
loop do
req = Net::HTTP::Head.new bottle_info.url
req.initialize_http_header "User-Agent" => HOMEBREW_USER_AGENT_RUBY
res = http.request req
@ -551,7 +556,7 @@ module Homebrew
max_curl_retries = 1
retry_count = 0
# We're in the cache; make sure to force re-download
while true do
loop do
begin
curl url, "-o", filename
break

55
Library/Homebrew/dev-cmd/test-bot.rb
View File

@ -120,7 +120,7 @@ module Homebrew
if git_url = ENV["UPSTREAM_GIT_URL"] || ENV["GIT_URL"]
# Also can get tap from Jenkins GIT_URL.
url_path = git_url.sub(%r{^https?://github\.com/}, "").chomp("/").sub(%r{\.git$}, "")
url_path = git_url.sub(%r{^https?://github\.com/}, "").chomp("/").sub(/\.git$/, "")
begin
return Tap.fetch(url_path) if url_path =~ HOMEBREW_TAP_REGEX
rescue
@ -190,7 +190,7 @@ module Homebrew
puts "#{Tty.white}==>#{Tty.red} FAILED#{Tty.reset}" if failed?
end
def has_output?
def output?
@output && !@output.empty?
end
@ -245,7 +245,6 @@ module Homebrew
@status = $?.success? ? :passed : :failed
puts_result
unless output.empty?
@output = Homebrew.fix_encoding!(output)
puts @output if (failed? || @puts_output_on_success) && !verbose
@ -259,7 +258,7 @@ module Homebrew
class Test
attr_reader :log_root, :category, :name, :steps
def initialize(argument, options={})
def initialize(argument, options = {})
@hash = nil
@url = nil
@formulae = []
@ -277,7 +276,7 @@ module Homebrew
elsif canonical_formula_name = safe_formula_canonical_name(argument)
@formulae = [canonical_formula_name]
else
raise ArgumentError.new("#{argument} is not a pull request URL, commit URL or formula name.")
raise ArgumentError, "#{argument} is not a pull request URL, commit URL or formula name."
end
@category = __method__
@ -404,7 +403,7 @@ module Homebrew
@short_url = @url.gsub("https://github.com/", "")
if @short_url.include? "/commit/"
# 7 characters should be enough for a commit (not 40).
@short_url.gsub!(/(commit\/\w{7}).*/, '\1')
@short_url.gsub!(%r{(commit/\w{7}).*/, '\1'})
@name = @short_url
else
@name = "#{@short_url}-#{diff_end_sha1}"
@ -569,7 +568,7 @@ module Homebrew
dependents -= @formulae
dependents = dependents.map { |d| Formulary.factory(d) }
bottled_dependents = dependents.select { |d| d.bottled? }
bottled_dependents = dependents.select(&:bottled?)
testable_dependents = dependents.select { |d| d.bottled? && d.test_defined? }
if (deps | reqs).any? { |d| d.name == "mercurial" && d.build? }
@ -630,9 +629,9 @@ module Homebrew
bottle_args << "--skip-relocation" if ARGV.include? "--skip-relocation"
test "brew", "bottle", *bottle_args
bottle_step = steps.last
if bottle_step.passed? && bottle_step.has_output?
if bottle_step.passed? && bottle_step.output?
bottle_filename =
bottle_step.output.gsub(/.*(\.\/\S+#{Utils::Bottles::native_regex}).*/m, '\1')
bottle_step.output.gsub(%r{.*(\./\S+#{Utils::Bottles.native_regex}).*/m, '\1'})
bottle_json_filename = bottle_filename.gsub(/\.(\d+\.)?tar\.gz$/, ".json")
bottle_merge_args = ["--merge", "--write", "--no-commit", bottle_json_filename]
bottle_merge_args << "--keep-old" if ARGV.include? "--keep-old"
@ -666,11 +665,10 @@ module Homebrew
next if steps.last.failed?
end
end
if dependent.installed?
test "brew", "linkage", "--test", dependent.name
if testable_dependents.include? dependent
test "brew", "test", "--verbose", dependent.name
end
next unless dependent.installed?
test "brew", "linkage", "--test", dependent.name
if testable_dependents.include? dependent
test "brew", "test", "--verbose", dependent.name
end
end
test "brew", "uninstall", "--force", formula_name
@ -795,7 +793,7 @@ module Homebrew
end
def test(*args)
options = Hash === args.last ? args.pop : {}
options = args.last.is_a?(Hash) ? args.pop : {}
options[:repository] = @repository
step = Step.new self, args, options
step.run
@ -934,7 +932,7 @@ module Homebrew
bintray_repo = bottle_hash["bintray"]["repository"]
bintray_repo_url = "https://api.bintray.com/packages/homebrew/#{bintray_repo}"
bottle_hash["bottle"]["tags"].each do |tag, tag_hash|
bottle_hash["bottle"]["tags"].each do |_tag, tag_hash|
filename = tag_hash["filename"]
if system "curl", "-I", "--silent", "--fail", "--output", "/dev/null",
"#{BottleSpecification::DEFAULT_DOMAIN}/#{bintray_repo}/#{filename}"
@ -973,7 +971,7 @@ module Homebrew
safe_system "git", "push", "--force", remote, "master:master", "refs/tags/#{git_tag}"
end
def sanitize_ARGV_and_ENV
def sanitize_argv_and_env
if Pathname.pwd == HOMEBREW_PREFIX && ARGV.include?("--cleanup")
odie "cannot use --cleanup from HOMEBREW_PREFIX as it will delete all output."
end
@ -1016,7 +1014,7 @@ module Homebrew
end
def test_bot
sanitize_ARGV_and_ENV
sanitize_argv_and_env
tap = resolve_test_tap
# Tap repository if required, this is done before everything else
@ -1070,19 +1068,18 @@ module Homebrew
testcase.add_attribute "status", step.status
testcase.add_attribute "time", step.time
if step.has_output?
output = sanitize_output_for_xml(step.output)
cdata = REXML::CData.new output
next unless step.output?
output = sanitize_output_for_xml(step.output)
cdata = REXML::CData.new output
if step.passed?
elem = testcase.add_element "system-out"
else
elem = testcase.add_element "failure"
elem.add_attribute "message", "#{step.status}: #{step.command.join(" ")}"
end
elem << cdata
if step.passed?
elem = testcase.add_element "system-out"
else
elem = testcase.add_element "failure"
elem.add_attribute "message", "#{step.status}: #{step.command.join(" ")}"
end
elem << cdata
end
end

12
Library/Homebrew/test/test_cmd_audit.rb
View File

@ -29,9 +29,9 @@ class FormulaTextTests < Homebrew::TestCase
def test_simple_valid_formula
ft = formula_text "valid", 'url "http://www.example.com/valid-1.0.tar.gz"'
refute ft.has_DATA?, "The formula should not have DATA"
refute ft.has_END?, "The formula should not have __END__"
assert ft.has_trailing_newline?, "The formula should have a trailing newline"
refute ft.data?, "The formula should not have DATA"
refute ft.end?, "The formula should not have __END__"
assert ft.trailing_newline?, "The formula should have a trailing newline"
assert ft =~ /\burl\b/, "The formula should match 'url'"
assert_nil ft.line_number(/desc/), "The formula should not match 'desc'"
@ -41,17 +41,17 @@ class FormulaTextTests < Homebrew::TestCase
def test_trailing_newline
ft = formula_text "newline"
assert ft.has_trailing_newline?, "The formula must have a trailing newline"
assert ft.trailing_newline?, "The formula must have a trailing newline"
end
def test_has_data
ft = formula_text "data", "patch :DATA"
assert ft.has_DATA?, "The formula must have DATA"
assert ft.data?, "The formula must have DATA"
end
def test_has_end
ft = formula_text "end", "", :patch => "__END__\na patch here"
assert ft.has_END?, "The formula must have __END__"
assert ft.end?, "The formula must have __END__"
assert_equal "class End < Formula\n \nend", ft.without_patch
end
end