diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index 417aee51ac..21eadabd30 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -1,5 +1,6 @@ require 'open-uri' require 'utils/json' +require 'erb' class AbstractDownloadStrategy attr_reader :name, :resource @@ -33,6 +34,14 @@ class AbstractDownloadStrategy safe_system(*expand_safe_system_args(args)) end + def checkout_name(tag) + if name.empty? || name == '__UNKNOWN__' + "#{ERB::Util.url_encode(@url)}--#{tag}" + else + "#{name}--#{tag}" + end + end + # All download strategies are expected to implement these methods def fetch; end def stage; end @@ -298,13 +307,11 @@ class SubversionDownloadStrategy < AbstractDownloadStrategy super @@svn ||= 'svn' - if name.to_s.empty? || name == '__UNKNOWN__' - raise NotImplementedError, "strategy requires a name parameter" + if ARGV.build_head? + @co = Pathname.new("#{HOMEBREW_CACHE}/#{checkout_name("svn-HEAD")}") else - @co = Pathname.new("#{HOMEBREW_CACHE}/#{name}--svn") + @co = Pathname.new("#{HOMEBREW_CACHE}/#{checkout_name("svn")}") end - - @co = Pathname.new(@co.to_s + '-HEAD') if ARGV.build_head? end def cached_location @@ -410,12 +417,7 @@ class GitDownloadStrategy < AbstractDownloadStrategy def initialize name, resource super @@git ||= 'git' - - if name.to_s.empty? || name == '__UNKNOWN__' - raise NotImplementedError, "strategy requires a name parameter" - else - @clone = Pathname.new("#{HOMEBREW_CACHE}/#{name}--git") - end + @clone = Pathname.new("#{HOMEBREW_CACHE}/#{checkout_name("git")}") end def cached_location @@ -562,13 +564,7 @@ end class CVSDownloadStrategy < AbstractDownloadStrategy def initialize name, resource super - - if name.to_s.empty? || name == '__UNKNOWN__' - raise NotImplementedError, "strategy requires a name parameter" - else - @unique_token = "#{name}--cvs" - @co = Pathname.new("#{HOMEBREW_CACHE}/#{@unique_token}") - end + @co = Pathname.new("#{HOMEBREW_CACHE}/#{checkout_name("cvs")}") end def cached_location; @co; end @@ -585,7 +581,7 @@ class CVSDownloadStrategy < AbstractDownloadStrategy unless @co.exist? Dir.chdir HOMEBREW_CACHE do safe_system '/usr/bin/cvs', '-d', url, 'login' - safe_system '/usr/bin/cvs', '-d', url, 'checkout', '-d', @unique_token, mod + safe_system '/usr/bin/cvs', '-d', url, 'checkout', '-d', checkout_name("cvs"), mod end else puts "Updating #{@co}" @@ -618,12 +614,7 @@ end class MercurialDownloadStrategy < AbstractDownloadStrategy def initialize name, resource super - - if name.to_s.empty? || name == '__UNKNOWN__' - raise NotImplementedError, "strategy requires a name parameter" - else - @clone = Pathname.new("#{HOMEBREW_CACHE}/#{name}--hg") - end + @clone = Pathname.new("#{HOMEBREW_CACHE}/#{checkout_name("hg")}") end def cached_location; @clone; end @@ -678,12 +669,7 @@ end class BazaarDownloadStrategy < AbstractDownloadStrategy def initialize name, resource super - - if name.to_s.empty? || name == '__UNKNOWN__' - raise NotImplementedError, "strategy requires a name parameter" - else - @clone = Pathname.new("#{HOMEBREW_CACHE}/#{name}--bzr") - end + @clone = Pathname.new("#{HOMEBREW_CACHE}/#{checkout_name("bzr")}") end def cached_location; @clone; end @@ -731,11 +717,7 @@ end class FossilDownloadStrategy < AbstractDownloadStrategy def initialize name, resource super - if name.to_s.empty? || name == '__UNKNOWN__' - raise NotImplementedError, "strategy requires a name parameter" - else - @clone = Pathname.new("#{HOMEBREW_CACHE}/#{name}--fossil") - end + @clone = Pathname.new("#{HOMEBREW_CACHE}/#{checkout_name("fossil")}") end def cached_location; @clone; end diff --git a/Library/Homebrew/test/test_download_strategies.rb b/Library/Homebrew/test/test_download_strategies.rb index 3a330e7976..cdcb92dc94 100644 --- a/Library/Homebrew/test/test_download_strategies.rb +++ b/Library/Homebrew/test/test_download_strategies.rb @@ -2,7 +2,7 @@ require 'testing_env' require 'download_strategy' require 'bottles' # XXX: hoist these regexps into constants in Pathname? -class SoftwareSpecDouble +class ResourceDouble attr_reader :url, :specs def initialize(url="http://foo.com/bar.tar.gz", specs={}) @@ -14,8 +14,8 @@ end class AbstractDownloadStrategyTests < Test::Unit::TestCase def setup @name = "foo" - @package = SoftwareSpecDouble.new - @strategy = AbstractDownloadStrategy.new(@name, @package) + @resource = ResourceDouble.new + @strategy = AbstractDownloadStrategy.new(@name, @resource) @args = %w{foo bar baz} end @@ -37,6 +37,32 @@ class AbstractDownloadStrategyTests < Test::Unit::TestCase end end +class DownloadStrategyCheckoutNameTests < Test::Unit::TestCase + def setup + @resource = ResourceDouble.new("http://foo.com/bar") + @strategy = AbstractDownloadStrategy + end + + def escaped(tag) + "#{ERB::Util.url_encode(@resource.url)}--#{tag}" + end + + def test_explicit_name + downloader = @strategy.new("baz", @resource) + assert_equal "baz--foo", downloader.checkout_name("foo") + end + + def test_empty_name + downloader = @strategy.new("", @resource) + assert_equal escaped("foo"), downloader.checkout_name("foo") + end + + def test_unknown_name + downloader = @strategy.new("__UNKNOWN__", @resource) + assert_equal escaped("foo"), downloader.checkout_name("foo") + end +end + class DownloadStrategyDetectorTests < Test::Unit::TestCase def setup @d = DownloadStrategyDetector.new