Merge pull request #1217 from MatzFan/invalid_build_option_warnings

Invalid build option warnings - supersedes #1088

Library/Homebrew/build_options.rb

@@ -101,6 +101,16 @@ class BuildOptions
     @options - @args
   end
 
+  # @private
+  def invalid_options
+    @args - @options
+  end
+
+  # @private
+  def invalid_option_names
+    invalid_options.map(&:flag).sort
+  end
+
   private
 
   def option_defined?(name)

Library/Homebrew/cmd/install.rb

@@ -249,19 +249,21 @@ module Homebrew
 
   def install_formula(f)
     f.print_tap_action
+    build_options = f.build
 
     fi = FormulaInstaller.new(f)
-    fi.options             = f.build.used_options
-    fi.ignore_deps         = ARGV.ignore_deps?
-    fi.only_deps           = ARGV.only_deps?
-    fi.build_bottle        = ARGV.build_bottle?
-    fi.build_from_source   = ARGV.build_from_source? || ARGV.build_all_from_source?
-    fi.force_bottle        = ARGV.force_bottle?
-    fi.interactive         = ARGV.interactive?
-    fi.git                 = ARGV.git?
-    fi.verbose             = ARGV.verbose?
-    fi.quieter             = ARGV.quieter?
-    fi.debug               = ARGV.debug?
+    fi.options              = build_options.used_options
+    fi.invalid_option_names = build_options.invalid_option_names
+    fi.ignore_deps          = ARGV.ignore_deps?
+    fi.only_deps            = ARGV.only_deps?
+    fi.build_bottle         = ARGV.build_bottle?
+    fi.build_from_source    = ARGV.build_from_source? || ARGV.build_all_from_source?
+    fi.force_bottle         = ARGV.force_bottle?
+    fi.interactive          = ARGV.interactive?
+    fi.git                  = ARGV.git?
+    fi.verbose              = ARGV.verbose?
+    fi.quieter              = ARGV.quieter?
+    fi.debug                = ARGV.debug?
     fi.prelude
     fi.install
     fi.finish

Library/Homebrew/formula_installer.rb

@@ -31,7 +31,7 @@ class FormulaInstaller
   end
 
   attr_reader :formula
-  attr_accessor :options, :build_bottle
+  attr_accessor :options, :build_bottle, :invalid_option_names
   mode_attr_accessor :show_summary_heading, :show_header
   mode_attr_accessor :build_from_source, :force_bottle
   mode_attr_accessor :ignore_deps, :only_deps, :interactive, :git
@@ -51,6 +51,7 @@ class FormulaInstaller
     @quieter = false
     @debug = false
     @options = Options.new
+    @invalid_option_names = []
 
     @@attempted ||= Set.new
 
@@ -213,6 +214,10 @@ class FormulaInstaller
       opoo "#{formula.full_name}: #{old_flag} was deprecated; using #{new_flag} instead!"
     end
 
+    invalid_option_names.each do |option|
+      opoo "#{formula.full_name}: this formula has no #{option} option so it will be ignored!"
+    end
+
     oh1 "Installing #{Formatter.identifier(formula.full_name)}" if show_header?
 
     if formula.tap && !formula.tap.private?

Library/Homebrew/test/test_build_options.rb

@@ -7,6 +7,8 @@ class BuildOptionsTests < Homebrew::TestCase
     args = Options.create(%w[--with-foo --with-bar --without-qux])
     opts = Options.create(%w[--with-foo --with-bar --without-baz --without-qux])
     @build = BuildOptions.new(args, opts)
+    bad_args = Options.create(%w[--with-foo --with-bar --without-bas --without-qux --without-abc])
+    @bad_build = BuildOptions.new(bad_args, opts)
   end
 
   def test_include
@@ -31,4 +33,17 @@ class BuildOptionsTests < Homebrew::TestCase
   def test_unused_options
     assert_includes @build.unused_options, "--without-baz"
   end
+
+  def test_invalid_options
+    assert_empty @build.invalid_options
+    assert_includes @bad_build.invalid_options, "--without-bas"
+    assert_includes @bad_build.invalid_options, "--without-abc"
+    refute_includes @bad_build.invalid_options, "--with-foo"
+    refute_includes @bad_build.invalid_options, "--with-baz"
+  end
+
+  def test_invalid_option_names
+    assert_empty @build.invalid_option_names
+    assert_equal @bad_build.invalid_option_names, %w[--without-abc --without-bas]
+  end
 end

Library/Homebrew/test/test_formula_installer.rb

@@ -37,6 +37,7 @@ class InstallTests < Homebrew::TestCase
   end
 
   def test_a_basic_install
+    ARGV << "--with-invalid_flag" # added to ensure it doesn't fail install
     temporary_install(Testball.new) do |f|
       # Test that things made it into the Keg
       assert_predicate f.prefix+"readme", :exist?

Library/Homebrew/test/test_install.rb

@@ -21,4 +21,10 @@ class IntegrationCommandTestInstall < IntegrationCommandTestCase
     assert_match "testball1 already installed, it's just not migrated",
       cmd("install", "testball2")
   end
+
+  def test_install_with_invalid_option
+    setup_test_formula "testball1"
+    assert_match "testball1: this formula has no --with-fo option so it will be ignored!",
+      cmd("install", "testball1", "--with-fo")
+  end
 end