From 62e14ea6733c9302889bc89820c7f88fbb593964 Mon Sep 17 00:00:00 2001 From: Bruce Steedman Date: Mon, 3 Oct 2016 09:42:53 +0100 Subject: [PATCH 1/5] invalid build options - fixed conflicts; rename --- Library/Homebrew/build_options.rb | 9 +++++++++ Library/Homebrew/cmd/install.rb | 4 +++- Library/Homebrew/formula_installer.rb | 7 ++++++- Library/Homebrew/test/test_build_options.rb | 15 +++++++++++++++ Library/Homebrew/test/test_formula_installer.rb | 1 + Library/Homebrew/test/test_install.rb | 6 ++++++ 6 files changed, 40 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/build_options.rb b/Library/Homebrew/build_options.rb index e9a06f4e00..1c44af147f 100644 --- a/Library/Homebrew/build_options.rb +++ b/Library/Homebrew/build_options.rb @@ -101,6 +101,15 @@ class BuildOptions @options - @args end + # @private + def invalid_options + @args - @options + end + + def invalid_option_names + invalid_options.map(&:flag).sort + end + private def option_defined?(name) diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 402a159db1..aa316b1870 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -261,7 +261,9 @@ module Homebrew f.print_tap_action fi = FormulaInstaller.new(f) - fi.options = f.build.used_options + build_options = f.build + fi.options = build_options.used_options + fi.invalid_opt_names = build_options.invalid_option_names fi.ignore_deps = ARGV.ignore_deps? fi.only_deps = ARGV.only_deps? fi.build_bottle = ARGV.build_bottle? diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 09c11318ee..f678acaf81 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -32,7 +32,7 @@ class FormulaInstaller end attr_reader :formula - attr_accessor :options, :build_bottle + attr_accessor :options, :build_bottle, :invalid_opt_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 @@ -52,6 +52,7 @@ class FormulaInstaller @quieter = false @debug = false @options = Options.new + @invalid_opt_names = [] @@attempted ||= Set.new @@ -214,6 +215,10 @@ class FormulaInstaller opoo "#{formula.full_name}: #{old_flag} was deprecated; using #{new_flag} instead!" end + invalid_opt_names.each do |option| + opoo "#{formula.full_name}: #{option} is invalid for this formula and will be ignored!" + end + oh1 "Installing #{Formatter.identifier(formula.full_name)}" if show_header? if formula.tap && !formula.tap.private? diff --git a/Library/Homebrew/test/test_build_options.rb b/Library/Homebrew/test/test_build_options.rb index dab418792a..02f4be0fa5 100644 --- a/Library/Homebrew/test/test_build_options.rb +++ b/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_opt_names + assert_empty @build.invalid_opt_names + assert_equal @bad_build.invalid_opt_names, %w[--without-abc --without-bas] + end end diff --git a/Library/Homebrew/test/test_formula_installer.rb b/Library/Homebrew/test/test_formula_installer.rb index 5b937d1df3..18bd910a67 100644 --- a/Library/Homebrew/test/test_formula_installer.rb +++ b/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? diff --git a/Library/Homebrew/test/test_install.rb b/Library/Homebrew/test/test_install.rb index 5d27d978b0..e0a40b5d75 100644 --- a/Library/Homebrew/test/test_install.rb +++ b/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: --with-fo is invalid for this formula and will be ignored!", + cmd("install", "testball1", "--with-fo") + end end From bbea4ac87123c8565d350d11bfae79de105c493c Mon Sep 17 00:00:00 2001 From: Bruce Steedman Date: Mon, 3 Oct 2016 09:53:20 +0100 Subject: [PATCH 2/5] fix bad method name in test --- Library/Homebrew/test/test_build_options.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/test/test_build_options.rb b/Library/Homebrew/test/test_build_options.rb index 02f4be0fa5..e460d25ccb 100644 --- a/Library/Homebrew/test/test_build_options.rb +++ b/Library/Homebrew/test/test_build_options.rb @@ -42,8 +42,8 @@ class BuildOptionsTests < Homebrew::TestCase refute_includes @bad_build.invalid_options, "--with-baz" end - def test_invalid_opt_names - assert_empty @build.invalid_opt_names - assert_equal @bad_build.invalid_opt_names, %w[--without-abc --without-bas] + 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 From 098974b2a12d48dd00f1ef77300d816fd867f259 Mon Sep 17 00:00:00 2001 From: Bruce Steedman Date: Sat, 12 Nov 2016 12:03:04 +0000 Subject: [PATCH 3/5] @MikeMcQuaid requested changes --- Library/Homebrew/build_options.rb | 1 + Library/Homebrew/formula_installer.rb | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/build_options.rb b/Library/Homebrew/build_options.rb index 1c44af147f..edcae01f53 100644 --- a/Library/Homebrew/build_options.rb +++ b/Library/Homebrew/build_options.rb @@ -106,6 +106,7 @@ class BuildOptions @args - @options end + # @private def invalid_option_names invalid_options.map(&:flag).sort end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index f678acaf81..15c1c332f1 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -32,7 +32,7 @@ class FormulaInstaller end attr_reader :formula - attr_accessor :options, :build_bottle, :invalid_opt_names + 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 @@ -52,7 +52,7 @@ class FormulaInstaller @quieter = false @debug = false @options = Options.new - @invalid_opt_names = [] + @invalid_option_names = [] @@attempted ||= Set.new @@ -215,8 +215,8 @@ class FormulaInstaller opoo "#{formula.full_name}: #{old_flag} was deprecated; using #{new_flag} instead!" end - invalid_opt_names.each do |option| - opoo "#{formula.full_name}: #{option} is invalid for this formula and will be ignored!" + 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? From 592905d3dabdda338520ea737e97ea273a127ff2 Mon Sep 17 00:00:00 2001 From: Bruce Steedman Date: Sat, 12 Nov 2016 12:13:27 +0000 Subject: [PATCH 4/5] fix test --- Library/Homebrew/test/test_install.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/test/test_install.rb b/Library/Homebrew/test/test_install.rb index e0a40b5d75..7135dddb2d 100644 --- a/Library/Homebrew/test/test_install.rb +++ b/Library/Homebrew/test/test_install.rb @@ -24,7 +24,7 @@ class IntegrationCommandTestInstall < IntegrationCommandTestCase def test_install_with_invalid_option setup_test_formula "testball1" - assert_match "testball1: --with-fo is invalid for this formula and will be ignored!", + assert_match "testball1: this formula has no --with-fo option so it will be ignored!", cmd("install", "testball1", "--with-fo") end end From 8ebddca0fed4cdbf3c3b8f9da42da2c10e5128c6 Mon Sep 17 00:00:00 2001 From: Bruce Steedman Date: Sat, 12 Nov 2016 12:31:35 +0000 Subject: [PATCH 5/5] fix other 10 failing tests - doh --- Library/Homebrew/cmd/install.rb | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index aa316b1870..d490b58508 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -259,21 +259,21 @@ module Homebrew def install_formula(f) f.print_tap_action + build_options = f.build fi = FormulaInstaller.new(f) - build_options = f.build - fi.options = build_options.used_options - fi.invalid_opt_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.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