Skip to content

Commit

Permalink
python: tweak script linking in virtualenv (Homebrew#613)
Browse files Browse the repository at this point in the history
* 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)
  • Loading branch information
UniqMartin authored and tdsmith committed Aug 2, 2016
1 parent a603352 commit b77a695
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 21 deletions.
22 changes: 11 additions & 11 deletions Library/Homebrew/language/python.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def virtualenv_create(venv_root, python = "python", formula = self)
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

Expand Down Expand Up @@ -218,18 +218,18 @@ def pip_install(targets)
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
Expand Down
31 changes: 21 additions & 10 deletions Library/Homebrew/test/test_language_python.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -71,18 +73,27 @@ def test_pip_install_accepts_resource
@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

0 comments on commit b77a695

Please sign in to comment.