FormulaInstaller: construct new ARGV from an Options collection

The array of options that is passed to the spawned build process is a
combination of the current ARGV, options passed in by a dependent
formula, and an existing install receipt. The objects that are
interacting here each expect the resulting collection to have certain
properties, and the expectations are not consistent.

Clear up this confusing mess by only dealing with Options collections.
This keeps our representation of options uniform across the codebase.

We can remove BuildOptions dependency on HomebrewArgvExtension, which
allows us to pass any Array-like collection to Tab.create. The only
other site inside of FormulaInstaller that uses the array is the #exec
call, and there it is splatted and thus we can substitute our Options
collection there as well.

2 files changed, 54 insertions, 1 deletions

diff --git a/Library/Homebrew/test/test_build_options.rb b/Library/Homebrew/test/test_build_options.rb
index cc73b28b1..daf5ef96b 100644
--- a/Library/Homebrew/test/test_build_options.rb
+++ b/Library/Homebrew/test/test_build_options.rb
@@ -3,7 +3,7 @@ require 'build_options'
 
 class BuildOptionsTests < Test::Unit::TestCase
   def setup
-    args = %w{--with-foo --with-bar --without-qux}.extend(HomebrewArgvExtension)
+    args = %w{--with-foo --with-bar --without-qux}
     @build = BuildOptions.new(args)
     @build.add("with-foo")
     @build.add("with-bar")
diff --git a/Library/Homebrew/test/test_options.rb b/Library/Homebrew/test/test_options.rb
index 065c00a82..78edb408c 100644
--- a/Library/Homebrew/test/test_options.rb
+++ b/Library/Homebrew/test/test_options.rb
@@ -38,6 +38,12 @@ class OptionTests < Test::Unit::TestCase
     assert_empty @option.description
     assert_equal "foo", Option.new("foo", "foo").description
   end
+
+  def test_preserves_short_options
+    option = Option.new("-d")
+    assert_equal "-d", option.flag
+    assert_equal "d", option.name
+  end
 end
 
 class OptionsTests < Test::Unit::TestCase
@@ -87,4 +93,51 @@ class OptionsTests < Test::Unit::TestCase
     @options << option
     assert_equal [option], @options.to_ary
   end
+
+  def test_concat_array
+    option = Option.new("foo")
+    @options.concat([option])
+    assert @options.include?(option)
+    assert_equal [option], @options.to_a
+  end
+
+  def test_concat_options
+    option = Option.new("foo")
+    opts = Options.new
+    opts << option
+    @options.concat(opts)
+    assert @options.include?(option)
+    assert_equal [option], @options.to_a
+  end
+
+  def test_concat_returns_self
+    assert_same @options, (@options.concat([]))
+  end
+
+  def test_intersection
+    foo, bar, baz = %w{foo bar baz}.map { |o| Option.new(o) }
+    options = Options.new << foo << bar
+    @options << foo << baz
+    assert_equal [foo], (@options & options).to_a
+  end
+
+  def test_coerce_with_options
+    assert_same @options, Options.coerce(@options)
+  end
+
+  def test_coerce_with_option
+    option = Option.new("foo")
+    assert_equal option, Options.coerce(option).to_a.first
+  end
+
+  def test_coerce_with_array
+    array = %w{--foo --bar}
+    option1 = Option.new("foo")
+    option2 = Option.new("bar")
+    assert_equal [option1, option2].sort, Options.coerce(array).to_a.sort
+  end
+
+  def test_coerce_raises_for_inappropriate_types
+    assert_raises(TypeError) { Options.coerce(1) }
+  end
 end