From 0c00201a6bd9a0bf3965167c936c07f7e5adfacc Mon Sep 17 00:00:00 2001 From: John Bender Date: Sun, 21 Mar 2010 23:50:08 -0700 Subject: [PATCH] switch to File.open for downloader temp file path to prevent minitar issues on windows --- .gitignore | 3 +- lib/vagrant/actions/box/download.rb | 137 +++++++++++----------- lib/vagrant/environment.rb | 2 +- test/vagrant/actions/box/download_test.rb | 6 +- test/vagrant/environment_test.rb | 9 +- 5 files changed, 84 insertions(+), 73 deletions(-) diff --git a/.gitignore b/.gitignore index 935e70502..53b43e7a6 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,5 @@ _site/* !templates/* *.org .yardoc/ -doc/ \ No newline at end of file +doc/ +*~ \ No newline at end of file diff --git a/lib/vagrant/actions/box/download.rb b/lib/vagrant/actions/box/download.rb index 89115727f..81e932e8e 100644 --- a/lib/vagrant/actions/box/download.rb +++ b/lib/vagrant/actions/box/download.rb @@ -1,66 +1,71 @@ -module Vagrant - module Actions - module Box - # An action which acts on a box by downloading the box file from - # the given URI into a temporary location. This action parses a - # given URI and handles downloading it via one of the many Vagrant - # downloads (such as {Vagrant::Downloaders::File}). - # - # This action cleans itself up by removing the downloaded box file. - class Download < Base - BASENAME = "box" - BUFFERSIZE = 1048576 # 1 MB - - attr_reader :downloader - - def prepare - # Parse the URI given and prepare a downloader - uri = URI.parse(@runner.uri) - uri_map = [[URI::HTTP, Downloaders::HTTP], [URI::Generic, Downloaders::File]] - - uri_map.find do |uri_type, downloader_klass| - if uri.is_a?(uri_type) - logger.info "#{uri_type} for URI, downloading via #{downloader_klass}..." - @downloader = downloader_klass.new - end - end - - raise ActionException.new(:box_download_unknown_type) unless @downloader - - # Prepare the downloader - @downloader.prepare(@runner.uri) - end - - def execute! - with_tempfile do |tempfile| - download_to(tempfile) - @runner.temp_path = tempfile.path - end - end - - def cleanup - if @runner.temp_path && File.exist?(@runner.temp_path) - logger.info "Cleaning up downloaded box..." - File.unlink(@runner.temp_path) - end - end - - def rescue(exception) - cleanup - end - - def with_tempfile - logger.info "Creating tempfile for storing box file..." - Tempfile.open(BASENAME, @runner.env.tmp_path) do |tempfile| - yield tempfile - end - end - - def download_to(f) - logger.info "Copying box to temporary location..." - downloader.download!(@runner.uri, f) - end - end - end - end -end \ No newline at end of file +module Vagrant + module Actions + module Box + # An action which acts on a box by downloading the box file from + # the given URI into a temporary location. This action parses a + # given URI and handles downloading it via one of the many Vagrant + # downloads (such as {Vagrant::Downloaders::File}). + # + # This action cleans itself up by removing the downloaded box file. + class Download < Base + BASENAME = "box" + BUFFERSIZE = 1048576 # 1 MB + + attr_reader :downloader + + def prepare + # Parse the URI given and prepare a downloader + uri = URI.parse(@runner.uri) + uri_map = [[URI::HTTP, Downloaders::HTTP], [URI::Generic, Downloaders::File]] + + uri_map.find do |uri_type, downloader_klass| + if uri.is_a?(uri_type) + logger.info "#{uri_type} for URI, downloading via #{downloader_klass}..." + @downloader = downloader_klass.new + end + end + + raise ActionException.new(:box_download_unknown_type) unless @downloader + + # Prepare the downloader + @downloader.prepare(@runner.uri) + end + + def execute! + with_tempfile do |tempfile| + download_to(tempfile) + @runner.temp_path = tempfile.path + end + end + + def cleanup + if @runner.temp_path && File.exist?(@runner.temp_path) + logger.info "Cleaning up downloaded box..." + File.unlink(@runner.temp_path) + end + end + + def rescue(exception) + cleanup + end + + def with_tempfile + logger.info "Creating tempfile for storing box file..." + # create, write only, fail if the file exists + File.open(file_temp_path, File::WRONLY|File::EXCL|File::CREAT) do |tempfile| + yield tempfile + end + end + + def file_temp_path + File.join(@runner.env.tmp_path, BASENAME + Time.now.to_i.to_s) + end + + def download_to(f) + logger.info "Copying box to temporary location..." + downloader.download!(@runner.uri, f) + end + end + end + end +end diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index ecc0bb924..8bc3534c9 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -260,4 +260,4 @@ module Vagrant error_and_exit(:environment_not_created) if !vm end end -end \ No newline at end of file +end diff --git a/test/vagrant/actions/box/download_test.rb b/test/vagrant/actions/box/download_test.rb index a416563bb..01ad99a55 100644 --- a/test/vagrant/actions/box/download_test.rb +++ b/test/vagrant/actions/box/download_test.rb @@ -81,13 +81,15 @@ class DownloadBoxActionTest < Test::Unit::TestCase context "tempfile" do should "create a tempfile in the vagrant tmp directory" do - Tempfile.expects(:open).with(Vagrant::Actions::Box::Download::BASENAME, @runner.env.tmp_path).once + File.expects(:open).with { |name, bitmask| + name =~ /#{Vagrant::Actions::Box::Download::BASENAME}/ && name =~ /#{@runner.env.tmp_path}/ + }.once @action.with_tempfile end should "yield the tempfile object" do @tempfile = mock("tempfile") - Tempfile.expects(:open).yields(@tempfile) + File.expects(:open).yields(@tempfile) @action.with_tempfile do |otherfile| assert @tempfile.equal?(otherfile) diff --git a/test/vagrant/environment_test.rb b/test/vagrant/environment_test.rb index 16f0afb97..50374cc12 100644 --- a/test/vagrant/environment_test.rb +++ b/test/vagrant/environment_test.rb @@ -153,12 +153,13 @@ class EnvironmentTest < Test::Unit::TestCase context "loading the root path" do setup do - @env.cwd = "/foo" + @env.cwd = "/foo" end should "default the path to the cwd instance var if nil" do @path = mock("path") @path.stubs(:root?).returns(true) + File.expects(:expand_path).with(@env.cwd).returns(@env.cwd) Pathname.expects(:new).with(@env.cwd).returns(@path) @env.load_root_path!(nil) end @@ -181,7 +182,8 @@ class EnvironmentTest < Test::Unit::TestCase search_seq = sequence("search_seq") paths.each do |path| - File.expects(:exist?).with("#{path}/#{Vagrant::Environment::ROOTFILE_NAME}").returns(false).in_sequence(search_seq) + # NOTE File.expect(:expand_path) causes tests to hang in windows below is the interim solution + File.expects(:exist?).with("#{File.expand_path(path)}/#{Vagrant::Environment::ROOTFILE_NAME}").returns(false).in_sequence(search_seq) end assert !@env.load_root_path!(paths.first) @@ -203,7 +205,8 @@ class EnvironmentTest < Test::Unit::TestCase end should "should set the path for the rootfile" do - path = "/foo" + # NOTE File.expect(:expand_path) causes tests to hang in windows below is the interim solution + path = File.expand_path("/foo") File.expects(:exist?).with("#{path}/#{Vagrant::Environment::ROOTFILE_NAME}").returns(true) assert @env.load_root_path!(Pathname.new(path))