python: tweak script linking in virtualenv (#613)

* python: tweak script linking in virtualenv

Instead of making the formula author use a slightly awkward block like

  venv.link_scripts(bin) { venv.pip_install buildpath }

avoid exposing this implementation detail and offer the more familiar:

  venv.pip_install buildpath, :link_scripts => bin

* Add non-block form and use instead of recursion

* Update 'pip_install' documentation

* Remove obsolete 'link_scripts'

* Add test for 'pip_install' with linking scripts

Also drop no longer relevant (and broken) `link_scripts` test, that
served as a template for the new test.

* Restore compatibility with Ruby 1.8.7

* Replace option hash with 'pip_install_and_link'

* Avoid confusing 'Object#tap' and fix silly bug

* Avoid side effects in mock object parameter check

* Simplify argument check (no need for a block)

2 files changed, 32 insertions, 21 deletions

diff --git a/Library/Homebrew/language/python.rb b/Library/Homebrew/language/python.rb
index 4249a911c..68e6f86ca 100644
--- a/Library/Homebrew/language/python.rb
+++ b/Library/Homebrew/language/python.rb
@@ -145,7 +145,7 @@ module Language
       def virtualenv_install_with_resources
         venv = virtualenv_create(libexec)
         venv.pip_install resources
-        venv.link_scripts(bin) { venv.pip_install buildpath }
+        venv.pip_install_and_link buildpath
         venv
       end
 
@@ -218,18 +218,18 @@ module Language
           end
         end
 
-        # Compares the venv bin directory before and after executing a block,
-        # and symlinks any new scripts into `destination`.
-        # Use like: venv.link_scripts(bin) { venv.pip_install my_package }
-        # @param destination [Pathname, String] Destination into which new
-        #   scripts should be linked.
-        # @return [void]
-        def link_scripts(destination)
+        # Installs packages represented by `targets` into the virtualenv, but
+        #   unlike {#pip_install} also links new scripts to {Formula#bin}.
+        # @param (see #pip_install)
+        # @return (see #pip_install)
+        def pip_install_and_link(targets)
           bin_before = Dir[@venv_root/"bin/*"].to_set
-          yield
+
+          pip_install(targets)
+
           bin_after = Dir[@venv_root/"bin/*"].to_set
-          destination = Pathname.new(destination)
-          destination.install_symlink((bin_after - bin_before).to_a)
+          bin_to_link = (bin_after - bin_before).to_a
+          @formula.bin.install_symlink(bin_to_link)
         end
 
         private
diff --git a/Library/Homebrew/test/test_language_python.rb b/Library/Homebrew/test/test_language_python.rb
index 5e91c73b0..d9d4f3458 100644
--- a/Library/Homebrew/test/test_language_python.rb
+++ b/Library/Homebrew/test/test_language_python.rb
@@ -6,8 +6,10 @@ class LanguagePythonTests < Homebrew::TestCase
   def setup
     @dir = Pathname.new(mktmpdir)
     resource = stub("resource", :stage => true)
+    formula_bin = @dir/"formula_bin"
     @formula = mock("formula") do
       stubs(:resource).returns(resource)
+      stubs(:bin).returns(formula_bin)
     end
     @venv = Language::Python::Virtualenv::Virtualenv.new(@formula, @dir, "python")
   end
@@ -71,18 +73,27 @@ class LanguagePythonTests < Homebrew::TestCase
     @venv.pip_install res
   end
 
-  def test_link_scripts_links_scripts
-    bin = (@dir/"bin")
-    dest = (@dir/"dest")
+  def test_pip_install_and_link_links_scripts
+    bin = @dir/"bin"
     bin.mkpath
-    refute((bin/"kilroy").exist?)
-    refute((dest/"kilroy").exist?)
+    dest = @formula.bin
+
+    refute_predicate bin/"kilroy", :exist?
+    refute_predicate dest/"kilroy", :exist?
+
     FileUtils.touch bin/"irrelevant"
-    @venv.link_scripts(dest) { FileUtils.touch bin/"kilroy" }
-    assert((bin/"kilroy").exist?)
-    assert((dest/"kilroy").exist?)
-    assert((dest/"kilroy").symlink?)
+    bin_before = Dir[bin/"*"]
+    FileUtils.touch bin/"kilroy"
+    bin_after = Dir[bin/"*"]
+    @venv.expects(:pip_install).with("foo")
+    Dir.expects(:[]).twice.returns(bin_before, bin_after)
+
+    @venv.pip_install_and_link "foo"
+
+    assert_predicate bin/"kilroy", :exist?
+    assert_predicate dest/"kilroy", :exist?
+    assert_predicate dest/"kilroy", :symlink?
     assert_equal((bin/"kilroy").realpath, (dest/"kilroy").realpath)
-    refute((dest/"irrelevant").exist?)
+    refute_predicate dest/"irrelevant", :exist?
   end
 end