From 4add1d1cb35ececf8823e2400a424254a44660ac Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Mon, 2 Sep 2019 10:50:49 +0100 Subject: [PATCH] Check binary URL resources with RuboCop - Migrate the existing binary URL audit to a RuboCop. - Check resources as well as main URLs - Also check for "macos" and "osx" in URLs - Add whitelists for URLs and formulae --- Library/Homebrew/dev-cmd/audit.rb | 12 ---- Library/Homebrew/rubocops/urls.rb | 34 +++++++++ Library/Homebrew/test/dev-cmd/audit_spec.rb | 78 --------------------- Library/Homebrew/test/rubocops/urls_spec.rb | 14 ++++ 4 files changed, 48 insertions(+), 90 deletions(-) diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 5276577b69..3817f062f7 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -920,18 +920,6 @@ module Homebrew EOS end - def audit_url_is_not_binary - return unless @core_tap - - urls = @specs.map(&:url) - - urls.each do |url| - if url =~ /darwin/i && (url =~ /x86_64/i || url =~ /amd64/i) - problem "#{url} looks like a binary package, not a source archive. The `core` tap is source-only." - end - end - end - def quote_dep(dep) dep.is_a?(Symbol) ? dep.inspect : "'#{dep}'" end diff --git a/Library/Homebrew/rubocops/urls.rb b/Library/Homebrew/rubocops/urls.rb index 774364d682..f5e2d65521 100644 --- a/Library/Homebrew/rubocops/urls.rb +++ b/Library/Homebrew/rubocops/urls.rb @@ -7,6 +7,28 @@ module RuboCop module FormulaAudit # This cop audits URLs and mirrors in Formulae. class Urls < FormulaCop + # These are formulae that, sadly, require an upstream binary to bootstrap. + BINARY_FORMULA_URLS_WHITELIST = %w[ + crystal + fpc + ghc + ghc@8.2 + go + go@1.9 + go@1.10 + go@1.11 + haskell-stack + ldc + mlton + rust + ].freeze + + # specific rust-nightly temporarily acceptable until a newer version is released. + # DO NOT RE-ADD A NEWER RUST-NIGHTLY IN FUTURE. + BINARY_URLS_WHITELIST = %w[ + https://static.rust-lang.org/dist/2019-08-24/rust-nightly-x86_64-apple-darwin.tar.xz + ].freeze + def audit_formula(_node, _class_node, _parent_class_node, body_node) urls = find_every_func_call_by_name(body_node, :url) mirrors = find_every_func_call_by_name(body_node, :mirror) @@ -202,6 +224,18 @@ module RuboCop audit_urls(urls, maven_pattern) do |match, url| problem "#{url} should be `https://search.maven.org/remotecontent?filepath=#{match[1]}`" end + + return if formula_tap != "homebrew-core" + + # Check for binary URLs + audit_urls(urls, /(darwin|macos|osx)/i) do |_, url| + next if url !~ /x86_64/i && url !~ /amd64/i + next if BINARY_FORMULA_URLS_WHITELIST.include?(@formula_name) + next if BINARY_URLS_WHITELIST.include?(url) + + problem "#{url} looks like a binary package, not a source archive. " \ + "Homebrew/homebrew-core is source-only." + end end end diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index 246749f969..73f88800e8 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -530,84 +530,6 @@ module Homebrew end end - describe "#audit_url_is_not_binary" do - specify "it detects a url containing darwin and x86_64" do - fa = formula_auditor "foo", <<~RUBY, core_tap: true - class Foo < Formula - url "https://brew.sh/example-darwin.x86_64.tar.gz" - end - RUBY - - fa.audit_url_is_not_binary - - expect(fa.problems.first) - .to match("looks like a binary package, not a source archive. The `core` tap is source-only.") - end - - specify "it detects a url containing darwin and amd64" do - fa = formula_auditor "foo", <<~RUBY, core_tap: true - class Foo < Formula - url "https://brew.sh/example-darwin.amd64.tar.gz" - end - RUBY - - fa.audit_url_is_not_binary - - expect(fa.problems.first) - .to match("looks like a binary package, not a source archive. The `core` tap is source-only.") - end - - specify "it works on the devel spec" do - fa = formula_auditor "foo", <<~RUBY, core_tap: true - class Foo < Formula - url "https://brew.sh/valid-1.0.tar.gz" - - devel do - url "https://brew.sh/example-darwin.x86_64.tar.gz" - end - end - RUBY - - fa.audit_url_is_not_binary - - expect(fa.problems.first) - .to match("looks like a binary package, not a source archive. The `core` tap is source-only.") - end - - specify "it works on the head spec" do - fa = formula_auditor "foo", <<~RUBY, core_tap: true - class Foo < Formula - url "https://brew.sh/valid-1.0.tar.gz" - - head do - url "https://brew.sh/example-darwin.x86_64.tar.gz" - end - end - RUBY - - fa.audit_url_is_not_binary - - expect(fa.problems.first) - .to match("looks like a binary package, not a source archive. The `core` tap is source-only.") - end - - specify "it ignores resource urls" do - fa = formula_auditor "foo", <<~RUBY, core_tap: true - class Foo < Formula - url "https://brew.sh/valid-1.0.tar.gz" - - resource "binary_res" do - url "https://brew.sh/example-darwin.x86_64.tar.gz" - end - end - RUBY - - fa.audit_url_is_not_binary - - expect(fa.problems).to eq([]) - end - end - describe "#audit_versioned_keg_only" do specify "it warns when a versioned formula is not `keg_only`" do fa = formula_auditor "foo@1.1", <<~RUBY, core_tap: true diff --git a/Library/Homebrew/test/rubocops/urls_spec.rb b/Library/Homebrew/test/rubocops/urls_spec.rb index 90f1603279..9b4a79f5e6 100644 --- a/Library/Homebrew/test/rubocops/urls_spec.rb +++ b/Library/Homebrew/test/rubocops/urls_spec.rb @@ -142,12 +142,26 @@ describe RuboCop::Cop::FormulaAudit::Urls do "msg" => "https://central.maven.org/maven2/com/bar/foo/1.1/foo-1.1.jar should be " \ "`https://search.maven.org/remotecontent?filepath=com/bar/foo/1.1/foo-1.1.jar`", "col" => 2, + }, { + "url" => "https://brew.sh/example-darwin.x86_64.tar.gz", + "msg" => "https://brew.sh/example-darwin.x86_64.tar.gz looks like a binary package, " \ + "not a source archive. Homebrew/homebrew-core is source-only.", + "col" => 2, + "formula_tap" => "homebrew-core", + }, { + "url" => "https://brew.sh/example-darwin.amd64.tar.gz", + "msg" => "https://brew.sh/example-darwin.amd64.tar.gz looks like a binary package, " \ + "not a source archive. Homebrew/homebrew-core is source-only.", + "col" => 2, + "formula_tap" => "homebrew-core", }] } context "When auditing urls" do it "with offenses" do formulae.each do |formula| + allow_any_instance_of(RuboCop::Cop::FormulaCop).to receive(:formula_tap) + .and_return(formula["formula_tap"]) source = <<~RUBY class Foo < Formula desc "foo"