From ffe30058ab2bfbd62c6e3a4e85df6c6cad389a31 Mon Sep 17 00:00:00 2001 From: Izaak Beekman Date: Thu, 6 Jun 2019 11:31:07 -0400 Subject: [PATCH 1/4] Add MPICH cop and test - Split off from PR Homebrew/brew#6209 - Create stand alone class for cop w/ auto-correct --- Library/Homebrew/rubocops/lines.rb | 18 ++++++++++++++++++ Library/Homebrew/test/rubocops/lines_spec.rb | 17 +++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index 3e9f8a9cc5a6f..31f84d57b9040 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -157,6 +157,24 @@ def unless_modifier?(node) end end + class MpiCheck < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + # Enforce use of OpenMPI for MPI dependency in core + return unless formula_tap == "homebrew-core" + + find_method_with_args(body_node, :depends_on, "mpich") do + problem "Use 'depends_on \"open-mpi\"' instead of '#{@offensive_node.source}'." + end + end + + def autocorrect(node) + # The dependency nodes may need to be re-sorted because of this + lambda do |corrector| + corrector.replace(node.source_range, "depends_on \"open-mpi\"") + end + end + end + class Miscellaneous < FormulaCop def audit_formula(_node, _class_node, _parent_class_node, body_node) # FileUtils is included in Formula diff --git a/Library/Homebrew/test/rubocops/lines_spec.rb b/Library/Homebrew/test/rubocops/lines_spec.rb index 2ad2497185559..161590f822a74 100644 --- a/Library/Homebrew/test/rubocops/lines_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_spec.rb @@ -279,6 +279,23 @@ def options end end +describe RuboCop::Cop::FormulaAudit::MpiCheck do + subject(:cop) { described_class.new } + + context "When auditing formula" do + it "reports an offense when using depends_on \"mpich\"" do + expect_offense(<<~RUBY, "/homebrew-core/") + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + depends_on "mpich" + ^^^^^^^^^^^^^^^^^^ Use 'depends_on "open-mpi"' instead of 'depends_on "mpich"'. + end + RUBY + end + end +end + describe RuboCop::Cop::FormulaAudit::Miscellaneous do subject(:cop) { described_class.new } From 65fbcc86d0c51c1ae04a6224b82d8fcf696632d3 Mon Sep 17 00:00:00 2001 From: Izaak Beekman Date: Thu, 6 Jun 2019 16:27:53 -0400 Subject: [PATCH 2/4] Add test for MPI choice cop's autocorrect --- Library/Homebrew/test/rubocops/lines_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Library/Homebrew/test/rubocops/lines_spec.rb b/Library/Homebrew/test/rubocops/lines_spec.rb index 161590f822a74..7f11573288ff0 100644 --- a/Library/Homebrew/test/rubocops/lines_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_spec.rb @@ -292,6 +292,14 @@ class Foo < Formula ^^^^^^^^^^^^^^^^^^ Use 'depends_on "open-mpi"' instead of 'depends_on "mpich"'. end RUBY + + expect_correction(<<~RUBY) + class Foo < Formula + desc "foo" + url 'https://brew.sh/foo-1.0.tgz' + depends_on "open-mpi" + end + RUBY end end end From 75bc30bfffa97ee7ed027e9fb3ed045fbc00961b Mon Sep 17 00:00:00 2001 From: Izaak Beekman Date: Thu, 6 Jun 2019 18:09:44 -0400 Subject: [PATCH 3/4] Limit veclibfort & lapack cop to core & add tests --- Library/Homebrew/rubocops/text.rb | 2 +- Library/Homebrew/test/rubocops/text_spec.rb | 22 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/rubocops/text.rb b/Library/Homebrew/rubocops/text.rb index e328e63585d47..7f695da721418 100644 --- a/Library/Homebrew/rubocops/text.rb +++ b/Library/Homebrew/rubocops/text.rb @@ -17,7 +17,7 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node) end if depends_on?("veclibfort") || depends_on?("lapack") - problem "Formulae should use OpenBLAS as the default serial linear algebra library." + problem "Formulae should use OpenBLAS as the default serial linear algebra library." if formula_tap == "homebrew-core" end if method_called_ever?(body_node, :virtualenv_create) || diff --git a/Library/Homebrew/test/rubocops/text_spec.rb b/Library/Homebrew/test/rubocops/text_spec.rb index 8485c5c4fec1c..f185fcbc9a8a1 100644 --- a/Library/Homebrew/test/rubocops/text_spec.rb +++ b/Library/Homebrew/test/rubocops/text_spec.rb @@ -32,6 +32,28 @@ class Foo < Formula RUBY end + it "when veclibfort is used instead of OpenBLAS" do + expect_offense(<<~RUBY, "/homebrew-core/") + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + homepage "https://brew.sh" + depends_on "veclibfort" + ^^^^^^^^^^^^^^^^^^^^^^^ Formulae should use OpenBLAS as the default serial linear algebra library. + end + RUBY + end + + it "when lapack is used instead of OpenBLAS" do + expect_offense(<<~RUBY, "/homebrew-core/") + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + homepage "https://brew.sh" + depends_on "lapack" + ^^^^^^^^^^^^^^^^^^^ Formulae should use OpenBLAS as the default serial linear algebra library. + end + RUBY + end + it "When xcodebuild is called without SYMROOT" do expect_offense(<<~RUBY) class Foo < Formula From 11de7de49d272af4f75c5289b28dfce7b735312c Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Fri, 7 Jun 2019 09:31:24 +0100 Subject: [PATCH 4/4] rubocops/text: fix long line. --- Library/Homebrew/rubocops/text.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/rubocops/text.rb b/Library/Homebrew/rubocops/text.rb index 7f695da721418..ec8130abb7e2a 100644 --- a/Library/Homebrew/rubocops/text.rb +++ b/Library/Homebrew/rubocops/text.rb @@ -16,8 +16,8 @@ def audit_formula(_node, _class_node, _parent_class_node, body_node) problem "Formulae should not depend on both OpenSSL and LibreSSL (even optionally)." end - if depends_on?("veclibfort") || depends_on?("lapack") - problem "Formulae should use OpenBLAS as the default serial linear algebra library." if formula_tap == "homebrew-core" + if formula_tap == "homebrew-core" && (depends_on?("veclibfort") || depends_on?("lapack")) + problem "Formulae should use OpenBLAS as the default serial linear algebra library." end if method_called_ever?(body_node, :virtualenv_create) ||