diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index de269d55eb..5d9d3f25c0 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -14,14 +14,13 @@ class AbstractDownloadStrategy end end - attr_reader :meta, :name, :version, :resource + attr_reader :meta, :name, :version attr_reader :shutup - def initialize(name, resource) + def initialize(name, version, resource) @name = name - @resource = resource @url = resource.url - @version = resource.version + @version = version @meta = resource.specs @shutup = false extend Pourable if meta[:bottle] @@ -91,7 +90,7 @@ end class VCSDownloadStrategy < AbstractDownloadStrategy REF_TYPES = [:tag, :branch, :revisions, :revision].freeze - def initialize(name, resource) + def initialize(name, version, resource) super @ref_type, @ref = extract_ref(meta) @revision = meta[:revision] @@ -137,7 +136,9 @@ class VCSDownloadStrategy < AbstractDownloadStrategy @clone end - delegate head?: :version + def head? + version.respond_to?(:head?) && version.head? + end # Return last commit's unique identifier for the repository. # Return most recent modified timestamp unless overridden. @@ -208,7 +209,7 @@ end class CurlDownloadStrategy < AbstractFileDownloadStrategy attr_reader :mirrors, :tarball_path, :temporary_path - def initialize(name, resource) + def initialize(name, version, resource) super @mirrors = resource.mirrors.dup @tarball_path = HOMEBREW_CACHE/"#{name}-#{version}#{ext}" @@ -255,6 +256,8 @@ class CurlDownloadStrategy < AbstractFileDownloadStrategy ohai "Downloading from #{url}" end + temporary_path.dirname.mkpath + curl_download resolved_url(url), to: temporary_path end @@ -386,7 +389,7 @@ class GitHubPrivateRepositoryDownloadStrategy < CurlDownloadStrategy require "utils/formatter" require "utils/github" - def initialize(name, resource) + def initialize(name, version, resource) super parse_url_pattern set_github_token @@ -492,7 +495,7 @@ end class ScpDownloadStrategy < AbstractFileDownloadStrategy attr_reader :tarball_path, :temporary_path - def initialize(name, resource) + def initialize(name, version, resource) super @tarball_path = HOMEBREW_CACHE/"#{name}-#{version}#{ext}" @temporary_path = Pathname.new("#{cached_location}.incomplete") @@ -543,7 +546,7 @@ class ScpDownloadStrategy < AbstractFileDownloadStrategy end class SubversionDownloadStrategy < VCSDownloadStrategy - def initialize(name, resource) + def initialize(name, version, resource) super @url = @url.sub("svn+http://", "") end @@ -628,7 +631,7 @@ class GitDownloadStrategy < VCSDownloadStrategy %r{http://llvm\.org}, ].freeze - def initialize(name, resource) + def initialize(name, version, resource) super @ref_type ||= :branch @ref ||= "master" @@ -800,7 +803,7 @@ class GitDownloadStrategy < VCSDownloadStrategy end class GitHubGitDownloadStrategy < GitDownloadStrategy - def initialize(name, resource) + def initialize(name, version, resource) super return unless %r{^https?://github\.com/(?[^/]+)/(?[^/]+)\.git$} =~ @url @@ -854,7 +857,7 @@ class GitHubGitDownloadStrategy < GitDownloadStrategy end class CVSDownloadStrategy < VCSDownloadStrategy - def initialize(name, resource) + def initialize(name, version, resource) super @url = @url.sub(%r{^cvs://}, "") @@ -924,7 +927,7 @@ class CVSDownloadStrategy < VCSDownloadStrategy end class MercurialDownloadStrategy < VCSDownloadStrategy - def initialize(name, resource) + def initialize(name, version, resource) super @url = @url.sub(%r{^hg://}, "") end @@ -980,7 +983,7 @@ class MercurialDownloadStrategy < VCSDownloadStrategy end class BazaarDownloadStrategy < VCSDownloadStrategy - def initialize(name, resource) + def initialize(name, version, resource) super @url.sub!(%r{^bzr://}, "") ENV["BZR_HOME"] = HOMEBREW_TEMP @@ -1031,7 +1034,7 @@ class BazaarDownloadStrategy < VCSDownloadStrategy end class FossilDownloadStrategy < VCSDownloadStrategy - def initialize(name, resource) + def initialize(name, version, resource) super @url = @url.sub(%r{^fossil://}, "") end diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index 17dbd6f9a5..974ed85cea 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -105,7 +105,7 @@ module Formulary formula_name = File.basename(bottle_name)[/(.+)-/, 1] resource = Resource.new(formula_name) { url bottle_name } resource.specs[:bottle] = true - downloader = CurlDownloadStrategy.new resource.name, resource + downloader = resource.downloader cached = downloader.cached_location.exist? downloader.fetch ohai "Pouring the cached bottle" if cached diff --git a/Library/Homebrew/resource.rb b/Library/Homebrew/resource.rb index e86f48ac1d..6343331a72 100644 --- a/Library/Homebrew/resource.rb +++ b/Library/Homebrew/resource.rb @@ -30,10 +30,6 @@ class Resource @resource.specs end - def version - @resource.version - end - def mirrors @resource.mirrors end @@ -57,7 +53,7 @@ class Resource end def downloader - download_strategy.new(download_name, Download.new(self)) + download_strategy.new(download_name, version, Download.new(self)) end # Removes /s from resource names; this allows go package names @@ -68,7 +64,9 @@ class Resource end def download_name - name.nil? ? owner.name : "#{owner.name}--#{escaped_name}" + return owner.name if name.nil? + return escaped_name if owner.nil? + "#{owner.name}--#{escaped_name}" end def cached_download diff --git a/Library/Homebrew/test/download_strategies_spec.rb b/Library/Homebrew/test/download_strategies_spec.rb index 66518015fa..d78ae2437c 100644 --- a/Library/Homebrew/test/download_strategies_spec.rb +++ b/Library/Homebrew/test/download_strategies_spec.rb @@ -1,11 +1,12 @@ require "download_strategy" describe AbstractDownloadStrategy do - subject { described_class.new(name, resource) } + subject { described_class.new(name, version, resource) } let(:specs) { {} } let(:name) { "foo" } let(:url) { "http://example.com/foo.tar.gz" } + let(:version) { nil } let(:resource) { double(Resource, url: url, mirrors: [], specs: specs, version: nil) } let(:args) { %w[foo bar baz] } @@ -35,22 +36,24 @@ end describe VCSDownloadStrategy do let(:url) { "http://example.com/bar" } - let(:resource) { double(Resource, url: url, mirrors: [], specs: {}, version: nil) } + let(:version) { nil } + let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) } describe "#cached_location" do it "returns the path of the cached resource" do allow_any_instance_of(described_class).to receive(:cache_tag).and_return("foo") - downloader = described_class.new("baz", resource) + downloader = described_class.new("baz", version, resource) expect(downloader.cached_location).to eq(HOMEBREW_CACHE/"baz--foo") end end end describe GitHubPrivateRepositoryDownloadStrategy do - subject { described_class.new("foo", resource) } + subject { described_class.new("foo", version, resource) } let(:url) { "https://github.com/owner/repo/archive/1.1.5.tar.gz" } - let(:resource) { double(Resource, url: url, mirrors: [], specs: {}, version: nil) } + let(:version) { nil } + let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) } before do ENV["HOMEBREW_GITHUB_API_TOKEN"] = "token" @@ -71,10 +74,11 @@ describe GitHubPrivateRepositoryDownloadStrategy do end describe GitHubPrivateRepositoryReleaseDownloadStrategy do - subject { described_class.new("foo", resource) } + subject { described_class.new("foo", version, resource) } let(:url) { "https://github.com/owner/repo/releases/download/tag/foo_v0.1.0_darwin_amd64.tar.gz" } - let(:resource) { double(Resource, url: url, mirrors: [], specs: {}, version: nil) } + let(:version) { nil } + let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) } before do ENV["HOMEBREW_GITHUB_API_TOKEN"] = "token" @@ -122,11 +126,12 @@ describe GitHubPrivateRepositoryReleaseDownloadStrategy do end describe GitHubGitDownloadStrategy do - subject { described_class.new(name, resource) } + subject { described_class.new(name, version, resource) } let(:name) { "brew" } let(:url) { "https://github.com/homebrew/brew.git" } - let(:resource) { double(Resource, url: url, mirrors: [], specs: {}, version: nil) } + let(:version) { nil } + let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) } it "parses the URL and sets the corresponding instance variables" do expect(subject.instance_variable_get(:@user)).to eq("homebrew") @@ -135,11 +140,12 @@ describe GitHubGitDownloadStrategy do end describe GitDownloadStrategy do - subject { described_class.new(name, resource) } + subject { described_class.new(name, version, resource) } let(:name) { "baz" } let(:url) { "https://github.com/homebrew/foo" } - let(:resource) { double(Resource, url: url, mirrors: [], specs: {}, version: nil) } + let(:version) { nil } + let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) } let(:cached_location) { subject.cached_location } before do @@ -202,14 +208,15 @@ describe GitDownloadStrategy do end describe S3DownloadStrategy do - subject { described_class.new(name, resource) } + subject { described_class.new(name, version, resource) } let(:name) { "foo" } let(:url) { "http://bucket.s3.amazonaws.com/foo.tar.gz" } - let(:resource) { double(Resource, url: url, mirrors: [], specs: {}, version: nil) } + let(:version) { nil } + let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) } describe "#_fetch" do - subject { described_class.new(name, resource)._fetch } + subject { described_class.new(name, version, resource)._fetch } context "when given Bad S3 URL" do let(:url) { "http://example.com/foo.tar.gz" } @@ -224,18 +231,19 @@ describe S3DownloadStrategy do end describe CurlDownloadStrategy do - subject { described_class.new(name, resource) } + subject { described_class.new(name, version, resource) } let(:name) { "foo" } let(:url) { "http://example.com/foo.tar.gz" } - let(:resource) { double(Resource, url: url, mirrors: [], specs: { user: "download:123456" }, version: nil) } + let(:version) { nil } + let(:resource) { double(Resource, url: url, mirrors: [], specs: { user: "download:123456" }) } it "parses the opts and sets the corresponding args" do expect(subject.send(:_curl_opts)).to eq(["--user", "download:123456"]) end describe "#tarball_path" do - subject { described_class.new(name, resource).tarball_path } + subject { described_class.new(name, version, resource).tarball_path } context "when URL ends with file" do it { is_expected.to eq(HOMEBREW_CACHE/"foo-.tar.gz") } @@ -251,12 +259,13 @@ end describe ScpDownloadStrategy do def resource_for(url) - double(Resource, url: url, mirrors: [], specs: {}, version: nil) + double(Resource, url: url, mirrors: [], specs: {}) end - subject { described_class.new(name, resource) } + subject { described_class.new(name, version, resource) } let(:name) { "foo" } let(:url) { "scp://example.com/foo.tar.gz" } + let(:version) { nil } let(:resource) { resource_for(url) } describe "#initialize" do @@ -271,7 +280,7 @@ describe ScpDownloadStrategy do context "with invalid URL #{invalid_url}" do it "raises ScpDownloadStrategyError" do expect { - described_class.new(name, resource_for(invalid_url)) + described_class.new(name, version, resource_for(invalid_url)) }.to raise_error(ScpDownloadStrategyError) end end