diff options
Diffstat (limited to 'Library/Homebrew/dev-cmd')
| -rw-r--r-- | Library/Homebrew/dev-cmd/audit.rb | 164 | ||||
| -rw-r--r-- | Library/Homebrew/dev-cmd/bottle.rb | 6 | ||||
| -rw-r--r-- | Library/Homebrew/dev-cmd/bump-formula-pr.rb | 15 | ||||
| -rw-r--r-- | Library/Homebrew/dev-cmd/pull.rb | 6 | ||||
| -rw-r--r-- | Library/Homebrew/dev-cmd/release-notes.rb | 4 |
5 files changed, 95 insertions, 100 deletions
diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 2c9336481..a7d498c0d 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -54,6 +54,7 @@ module Homebrew def audit Homebrew.inject_dump_stats!(FormulaAuditor, /^audit_/) if ARGV.switch? "D" + Homebrew.auditing = true formula_count = 0 problem_count = 0 @@ -201,19 +202,24 @@ class FormulaAuditor @specs = %w[stable devel head].map { |s| formula.send(s) }.compact end - def self.check_http_content(url, name, user_agents: [:default]) + 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:") && name != "curl" + 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] + # Hack around https://github.com/Homebrew/brew/issues/3199 + return if MacOS.version == :el_capitan + return "The URL #{url} is not reachable" + end + unless details[:status].start_with? "2" return "The URL #{url} is not reachable (HTTP status code #{details[:status]})" end @@ -236,8 +242,32 @@ class FormulaAuditor details[:content_length] == secure_details[:content_length] file_match = details[:file_hash] == secure_details[:file_hash] - return if !etag_match && !content_length_match && !file_match - "The URL #{url} could use HTTPS rather than HTTP" + 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) @@ -260,6 +290,7 @@ class FormulaAuditor etag: headers[%r{ETag: ([wW]\/)?"(([^"]|\\")*)"}, 2], content_length: headers[/Content-Length: (\d+)/, 1], file_hash: output_hash, + file: output, } end @@ -327,7 +358,7 @@ class FormulaAuditor end valid_alias_names = [alias_name_major, alias_name_major_minor] - if formula.tap && !formula.tap.core_tap? + unless formula.tap&.core_tap? versioned_aliases.map! { |a| "#{formula.tap}/#{a}" } valid_alias_names.map! { |a| "#{formula.tap}/#{a}" } end @@ -356,21 +387,6 @@ class FormulaAuditor end end - def audit_class - if @strict - unless formula.test_defined? - problem "A `test do` test block should be added" - end - end - - classes = %w[GithubGistFormula ScriptFileFormula AmazonWebServicesFormula] - klass = classes.find do |c| - Object.const_defined?(c) && formula.class < Object.const_get(c) - end - - problem "#{klass} is deprecated, use Formula instead" if klass - end - # core aliases + tap alias names + tap alias full name @@aliases ||= Formula.aliases + Formula.tap_aliases @@ -403,6 +419,7 @@ class FormulaAuditor @@local_official_taps_name_map ||= Tap.select(&:official?).flat_map(&:formula_names) .each_with_object({}) do |tap_formula_full_name, name_map| + next if tap_formula_full_name.start_with?("homebrew/science/") tap_formula_name = tap_formula_full_name.split("/").last name_map[tap_formula_name] ||= [] name_map[tap_formula_name] << tap_formula_full_name @@ -413,6 +430,7 @@ class FormulaAuditor if @online Homebrew.search_taps(name, silent: true).each do |tap_formula_full_name| + next if tap_formula_full_name.start_with?("homebrew/science/") tap_formula_name = tap_formula_full_name.split("/").last next if tap_formula_name != name same_name_tap_formulae << tap_formula_full_name @@ -563,10 +581,11 @@ class FormulaAuditor return unless @online - return unless DevelopmentTools.curl_handles_most_https_homepages? + return unless DevelopmentTools.curl_handles_most_https_certificates? if http_content_problem = FormulaAuditor.check_http_content(homepage, - formula.name, - user_agents: [:browser, :default]) + user_agents: [:browser, :default], + check_content: true, + strict: @strict) problem http_content_problem end end @@ -616,13 +635,14 @@ class FormulaAuditor end %w[Stable Devel HEAD].each do |name| - next unless spec = formula.send(name.downcase) + spec_name = name.downcase.to_sym + next unless spec = formula.send(spec_name) - ra = ResourceAuditor.new(spec, online: @online, strict: @strict).audit + 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, online: @online, strict: @strict).audit + ra = ResourceAuditor.new(resource, spec_name, online: @online, strict: @strict).audit problems.concat ra.problems.map { |problem| "#{name} resource #{resource.name.inspect}: #{problem}" } @@ -687,7 +707,7 @@ class FormulaAuditor end stable = formula.stable - case stable && stable.url + case stable&.url when /[\d\._-](alpha|beta|rc\d)/ matched = Regexp.last_match(1) version_prefix = stable.version.to_s.sub(/\d+$/, "") @@ -865,8 +885,8 @@ class FormulaAuditor problem "Use spaces instead of tabs for indentation" if line =~ /^[ ]*\t/ - if line.include?("ENV.x11") - problem "Use \"depends_on :x11\" instead of \"ENV.x11\"" + if line.include?("ENV.java_cache") + problem "In-formula ENV.java_cache usage has been deprecated & should be removed." end # Avoid hard-coding compilers @@ -882,14 +902,6 @@ class FormulaAuditor problem "Use ENV instead of invoking '#{Regexp.last_match(1)}' to modify the environment" end - if formula.name != "wine" && line =~ /ENV\.universal_binary/ - problem "macOS has been 64-bit only since 10.6 so ENV.universal_binary is deprecated." - end - - if line =~ /build\.universal\?/ - problem "macOS has been 64-bit only so build.universal? is deprecated." - end - if line =~ /version == ['"]HEAD['"]/ problem "Use 'build.head?' instead of inspecting 'version'" end @@ -930,12 +942,6 @@ class FormulaAuditor problem "Use build instead of ARGV to check options" end - problem "Use new-style option definitions" if line.include?("def options") - - if line.end_with?("def test") - problem "Use new-style test definitions (test do)" - end - if line.include?("MACOS_VERSION") problem "Use MacOS.version instead of MACOS_VERSION" end @@ -949,11 +955,6 @@ class FormulaAuditor 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 @@ -992,30 +993,6 @@ class FormulaAuditor problem "Use `assert_match` instead of `assert ...include?`" end - if line.include?('system "npm", "install"') && !line.include?("Language::Node") && - formula.name !~ /^kibana(\@\d+(\.\d+)?)?$/ - problem "Use Language::Node for npm install args" - end - - if line.include?("fails_with :llvm") - problem "'fails_with :llvm' is now a no-op so should be removed" - end - - if line =~ /system\s+['"](otool|install_name_tool|lipo)/ && formula.name != "cctools" - problem "Use ruby-macho instead of calling #{Regexp.last_match(1)}" - end - - if formula.tap.to_s == "homebrew/core" - ["OS.mac?", "OS.linux?"].each do |check| - next unless line.include?(check) - problem "Don't use #{check}; Homebrew/core only supports macOS" - end - end - - if line =~ /((revision|version_scheme)\s+0)/ - problem "'#{Regexp.last_match(1)}' should be removed" - end - return unless @strict problem "`#{Regexp.last_match(1)}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/ @@ -1041,7 +1018,7 @@ class FormulaAuditor 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&.official? return unless formula.tap.tap_migrations.key?(formula.name) problem <<-EOS.undent @@ -1116,10 +1093,10 @@ class FormulaAuditor end class ResourceAuditor - attr_reader :problems - attr_reader :version, :checksum, :using, :specs, :url, :mirrors, :name + attr_reader :name, :version, :checksum, :url, :mirrors, :using, :specs, :owner + attr_reader :spec_name, :problems - def initialize(resource, options = {}) + def initialize(resource, spec_name, options = {}) @name = resource.name @version = resource.version @checksum = resource.checksum @@ -1127,9 +1104,11 @@ class ResourceAuditor @mirrors = resource.mirrors @using = resource.using @specs = resource.specs - @online = options[:online] - @strict = options[:strict] - @problems = [] + @owner = resource.owner + @spec_name = spec_name + @online = options[:online] + @strict = options[:strict] + @problems = [] end def audit @@ -1203,11 +1182,29 @@ class ResourceAuditor problem "Redundant :using value in URL" end + def self.curl_openssl_and_deps + @curl_openssl_and_deps ||= begin + formulae_names = ["curl", "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 - if name == "curl" && !urls.find { |u| u.start_with?("http://") } - problem "should always include at least one HTTP url" + curl_openssl_or_deps = ResourceAuditor.curl_openssl_and_deps.include?(owner.name) + + if spec_name == :stable && curl_openssl_or_deps + problem "should not use xz tarballs" if url.end_with?(".xz") + + unless urls.find { |u| u.start_with?("http://") } + problem "should always include at least one HTTP mirror" + end end return unless @online @@ -1219,7 +1216,7 @@ class ResourceAuditor # 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) + if http_content_problem = FormulaAuditor.check_http_content(url, require_http: curl_openssl_or_deps) problem http_content_problem end elsif strategy <= GitDownloadStrategy @@ -1228,6 +1225,7 @@ class ResourceAuditor 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 diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index d8aefc4c0..8dfd0d12c 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/Library/Homebrew/dev-cmd/bottle.rb @@ -47,7 +47,7 @@ BOTTLE_ERB = <<-EOS.freeze <% elsif cellar != BottleSpecification::DEFAULT_CELLAR %> cellar "<%= cellar %>" <% end %> - <% if rebuild > 0 %> + <% if rebuild.positive? %> rebuild <%= rebuild %> <% end %> <% checksums.each do |checksum_type, checksum_values| %> @@ -186,7 +186,7 @@ module Homebrew ohai "Determining #{f.full_name} bottle rebuild..." versions = FormulaVersions.new(f) rebuilds = versions.bottle_version_map("origin/master")[f.pkg_version] - rebuilds.pop if rebuilds.last.to_i > 0 + rebuilds.pop if rebuilds.last.to_i.positive? rebuild = rebuilds.empty? ? 0 : rebuilds.max.to_i + 1 end @@ -283,7 +283,7 @@ module Homebrew raise ensure ignore_interrupts do - original_tab.write if original_tab + original_tab&.write unless ARGV.include? "--skip-relocation" keg.replace_placeholders_with_locations changed_files end diff --git a/Library/Homebrew/dev-cmd/bump-formula-pr.rb b/Library/Homebrew/dev-cmd/bump-formula-pr.rb index e9e98d450..87d8274cc 100644 --- a/Library/Homebrew/dev-cmd/bump-formula-pr.rb +++ b/Library/Homebrew/dev-cmd/bump-formula-pr.rb @@ -89,7 +89,8 @@ module Homebrew def check_for_duplicate_pull_requests(formula) pull_requests = fetch_pull_requests(formula) - return unless pull_requests && !pull_requests.empty? + return unless pull_requests + return if pull_requests.empty? duplicates_message = <<-EOS.undent These open pull requests may be duplicates: #{pull_requests.map { |pr| "#{pr["title"]} #{pr["html_url"]}" }.join("\n")} @@ -124,7 +125,7 @@ module Homebrew Formula.each do |f| if is_devel && f.devel && f.devel.url && f.devel.url.match(base_url) guesses << f - elsif f.stable && f.stable.url && f.stable.url.match(base_url) + elsif f.stable&.url && f.stable.url.match(base_url) guesses << f end end @@ -296,9 +297,7 @@ module Homebrew ohai "git fetch --unshallow origin" if shallow ohai "git checkout --no-track -b #{branch} origin/master" ohai "git commit --no-edit --verbose --message='#{formula.name} #{new_formula_version}#{devel_message}' -- #{formula.path}" - ohai "hub fork --no-remote" - ohai "hub fork" - ohai "hub fork (to read $HUB_REMOTE)" + ohai "hub fork # read $HUB_REMOTE" ohai "git push --set-upstream $HUB_REMOTE #{branch}:#{branch}" ohai "hub pull-request --browse -m '#{formula.name} #{new_formula_version}#{devel_message}'" ohai "git checkout -" @@ -308,9 +307,9 @@ module Homebrew safe_system "git", "commit", "--no-edit", "--verbose", "--message=#{formula.name} #{new_formula_version}#{devel_message}", "--", formula.path - safe_system "hub", "fork", "--no-remote" - quiet_system "hub", "fork" - remote = Utils.popen_read("hub fork 2>&1")[/fatal: remote (.+) already exists\./, 1] + remote = Utils.popen_read("hub fork 2>&1")[/remote:? (\S+)/, 1] + # repeat for hub 2.2 backwards compatibility: + remote = Utils.popen_read("hub fork 2>&1")[/remote:? (\S+)/, 1] if remote.to_s.empty? odie "cannot get remote from 'hub'!" if remote.to_s.empty? safe_system "git", "push", "--set-upstream", remote, "#{branch}:#{branch}" pr_message = <<-EOS.undent diff --git a/Library/Homebrew/dev-cmd/pull.rb b/Library/Homebrew/dev-cmd/pull.rb index a8f35531f..cd0d6fbd0 100644 --- a/Library/Homebrew/dev-cmd/pull.rb +++ b/Library/Homebrew/dev-cmd/pull.rb @@ -69,13 +69,13 @@ module Homebrew tap = nil ARGV.named.each do |arg| - if arg.to_i > 0 + if arg.to_i.positive? issue = arg url = "https://github.com/Homebrew/homebrew-core/pull/#{arg}" tap = CoreTap.instance elsif (testing_match = arg.match %r{/job/Homebrew.*Testing/(\d+)/}) tap = ARGV.value("tap") - tap = if tap && tap.start_with?("homebrew/") + tap = if tap&.start_with?("homebrew/") Tap.fetch("homebrew", tap.strip_prefix("homebrew/")) elsif tap odie "Tap option did not start with \"homebrew/\": #{tap}" @@ -350,7 +350,7 @@ module Homebrew files << Regexp.last_match(1) if line =~ %r{^\+\+\+ b/(.*)} end files.each do |file| - if tap && tap.formula_file?(file) + if tap&.formula_file?(file) formula_name = File.basename(file, ".rb") formulae << formula_name unless formulae.include?(formula_name) else diff --git a/Library/Homebrew/dev-cmd/release-notes.rb b/Library/Homebrew/dev-cmd/release-notes.rb index e578869bf..496023956 100644 --- a/Library/Homebrew/dev-cmd/release-notes.rb +++ b/Library/Homebrew/dev-cmd/release-notes.rb @@ -10,10 +10,8 @@ module Homebrew def release_notes previous_tag = ARGV.named.first - unless previous_tag - previous_tag = Utils.popen_read("git tag --list --sort=-version:refname") + previous_tag ||= Utils.popen_read("git tag --list --sort=-version:refname") .lines.first.chomp - end odie "Could not find any previous tags!" unless previous_tag end_ref = ARGV.named[1] || "origin/master" |