Explicitly pass url and mirrors to download strategy.

Library/Homebrew/download_strategy.rb

@@ -17,12 +17,13 @@ class AbstractDownloadStrategy
   attr_reader :meta, :name, :version
   attr_reader :shutup
 
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
+    @url = url
     @name = name
-    @url = resource.url
     @version = version
     @meta = resource.specs
     @shutup = false
+    @mirrors = mirrors
     extend Pourable if meta[:bottle]
   end
 
@@ -90,7 +91,7 @@ end
 class VCSDownloadStrategy < AbstractDownloadStrategy
   REF_TYPES = [:tag, :branch, :revisions, :revision].freeze
 
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     @ref_type, @ref = extract_ref(meta)
     @revision = meta[:revision]
@@ -209,9 +210,8 @@ end
 class CurlDownloadStrategy < AbstractFileDownloadStrategy
   attr_reader :mirrors, :tarball_path, :temporary_path
 
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
-    @mirrors = resource.mirrors.dup
     @tarball_path = HOMEBREW_CACHE/"#{name}-#{version}#{ext}"
     @temporary_path = Pathname.new("#{cached_location}.incomplete")
   end
@@ -389,7 +389,7 @@ class GitHubPrivateRepositoryDownloadStrategy < CurlDownloadStrategy
   require "utils/formatter"
   require "utils/github"
 
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     parse_url_pattern
     set_github_token
@@ -495,7 +495,7 @@ end
 class ScpDownloadStrategy < AbstractFileDownloadStrategy
   attr_reader :tarball_path, :temporary_path
 
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     @tarball_path = HOMEBREW_CACHE/"#{name}-#{version}#{ext}"
     @temporary_path = Pathname.new("#{cached_location}.incomplete")
@@ -546,7 +546,7 @@ class ScpDownloadStrategy < AbstractFileDownloadStrategy
 end
 
 class SubversionDownloadStrategy < VCSDownloadStrategy
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     @url = @url.sub("svn+http://", "")
   end
@@ -631,7 +631,7 @@ class GitDownloadStrategy < VCSDownloadStrategy
     %r{http://llvm\.org},
   ].freeze
 
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     @ref_type ||= :branch
     @ref ||= "master"
@@ -803,7 +803,7 @@ class GitDownloadStrategy < VCSDownloadStrategy
 end
 
 class GitHubGitDownloadStrategy < GitDownloadStrategy
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
 
     return unless %r{^https?://github\.com/(?<user>[^/]+)/(?<repo>[^/]+)\.git$} =~ @url
@@ -857,7 +857,7 @@ class GitHubGitDownloadStrategy < GitDownloadStrategy
 end
 
 class CVSDownloadStrategy < VCSDownloadStrategy
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     @url = @url.sub(%r{^cvs://}, "")
 
@@ -927,7 +927,7 @@ class CVSDownloadStrategy < VCSDownloadStrategy
 end
 
 class MercurialDownloadStrategy < VCSDownloadStrategy
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     @url = @url.sub(%r{^hg://}, "")
   end
@@ -983,7 +983,7 @@ class MercurialDownloadStrategy < VCSDownloadStrategy
 end
 
 class BazaarDownloadStrategy < VCSDownloadStrategy
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     @url.sub!(%r{^bzr://}, "")
     ENV["BZR_HOME"] = HOMEBREW_TEMP
@@ -1034,7 +1034,7 @@ class BazaarDownloadStrategy < VCSDownloadStrategy
 end
 
 class FossilDownloadStrategy < VCSDownloadStrategy
-  def initialize(name, version, resource)
+  def initialize(url, name, version, resource, mirrors: [])
     super
     @url = @url.sub(%r{^fossil://}, "")
   end

Library/Homebrew/resource.rb

@@ -22,17 +22,9 @@ class Resource
       @resource = resource
     end
 
-    def url
-      @resource.url
-    end
-
     def specs
       @resource.specs
     end
-
-    def mirrors
-      @resource.mirrors
-    end
   end
 
   def initialize(name = nil, &block)
@@ -53,7 +45,8 @@ class Resource
   end
 
   def downloader
-    download_strategy.new(download_name, version, Download.new(self))
+    download_strategy.new(url, download_name, version, Download.new(self),
+                          mirrors: mirrors.dup)
   end
 
   # Removes /s from resource names; this allows go package names

Library/Homebrew/test/download_strategies_spec.rb

@@ -1,13 +1,13 @@
 require "download_strategy"
 
 describe AbstractDownloadStrategy do
-  subject { described_class.new(name, version, resource) }
+  subject { described_class.new(url, 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(:resource) { double(Resource, specs: specs) }
   let(:args) { %w[foo bar baz] }
 
   specify "#source_modified_time" do
@@ -37,23 +37,23 @@ end
 describe VCSDownloadStrategy do
   let(:url) { "http://example.com/bar" }
   let(:version) { nil }
-  let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) }
+  let(:resource) { double(Resource, 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", version, resource)
+      downloader = described_class.new(url, "baz", version, resource)
       expect(downloader.cached_location).to eq(HOMEBREW_CACHE/"baz--foo")
     end
   end
 end
 
 describe GitHubPrivateRepositoryDownloadStrategy do
-  subject { described_class.new("foo", version, resource) }
+  subject { described_class.new(url, "foo", version, resource) }
 
   let(:url) { "https://github.com/owner/repo/archive/1.1.5.tar.gz" }
   let(:version) { nil }
-  let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) }
+  let(:resource) { double(Resource, specs: {}) }
 
   before do
     ENV["HOMEBREW_GITHUB_API_TOKEN"] = "token"
@@ -74,11 +74,11 @@ describe GitHubPrivateRepositoryDownloadStrategy do
 end
 
 describe GitHubPrivateRepositoryReleaseDownloadStrategy do
-  subject { described_class.new("foo", version, resource) }
+  subject { described_class.new(url, "foo", version, resource) }
 
   let(:url) { "https://github.com/owner/repo/releases/download/tag/foo_v0.1.0_darwin_amd64.tar.gz" }
   let(:version) { nil }
-  let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) }
+  let(:resource) { double(Resource, url: url, specs: {}) }
 
   before do
     ENV["HOMEBREW_GITHUB_API_TOKEN"] = "token"
@@ -126,12 +126,12 @@ describe GitHubPrivateRepositoryReleaseDownloadStrategy do
 end
 
 describe GitHubGitDownloadStrategy do
-  subject { described_class.new(name, version, resource) }
+  subject { described_class.new(url, name, version, resource) }
 
   let(:name) { "brew" }
   let(:url) { "https://github.com/homebrew/brew.git" }
   let(:version) { nil }
-  let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) }
+  let(:resource) { double(Resource, specs: {}) }
 
   it "parses the URL and sets the corresponding instance variables" do
     expect(subject.instance_variable_get(:@user)).to eq("homebrew")
@@ -140,12 +140,12 @@ describe GitHubGitDownloadStrategy do
 end
 
 describe GitDownloadStrategy do
-  subject { described_class.new(name, version, resource) }
+  subject { described_class.new(url, name, version, resource) }
 
   let(:name) { "baz" }
   let(:url) { "https://github.com/homebrew/foo" }
   let(:version) { nil }
-  let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) }
+  let(:resource) { double(Resource, specs: {}) }
   let(:cached_location) { subject.cached_location }
 
   before do
@@ -208,15 +208,15 @@ describe GitDownloadStrategy do
 end
 
 describe S3DownloadStrategy do
-  subject { described_class.new(name, version, resource) }
+  subject { described_class.new(url, name, version, resource) }
 
   let(:name) { "foo" }
   let(:url) { "http://bucket.s3.amazonaws.com/foo.tar.gz" }
   let(:version) { nil }
-  let(:resource) { double(Resource, url: url, mirrors: [], specs: {}) }
+  let(:resource) { double(Resource, specs: {}) }
 
   describe "#_fetch" do
-    subject { described_class.new(name, version, resource)._fetch }
+    subject { described_class.new(url, name, version, resource)._fetch }
 
     context "when given Bad S3 URL" do
       let(:url) { "http://example.com/foo.tar.gz" }
@@ -231,19 +231,19 @@ describe S3DownloadStrategy do
 end
 
 describe CurlDownloadStrategy do
-  subject { described_class.new(name, version, resource) }
+  subject { described_class.new(url, name, version, resource) }
 
   let(:name) { "foo" }
   let(:url) { "http://example.com/foo.tar.gz" }
   let(:version) { nil }
-  let(:resource) { double(Resource, url: url, mirrors: [], specs: { user: "download:123456" }) }
+  let(:resource) { double(Resource, 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, version, resource).tarball_path }
+    subject { described_class.new(url, name, version, resource).tarball_path }
 
     context "when URL ends with file" do
       it { is_expected.to eq(HOMEBREW_CACHE/"foo-.tar.gz") }
@@ -258,15 +258,11 @@ describe CurlDownloadStrategy do
 end
 
 describe ScpDownloadStrategy do
-  def resource_for(url)
+  subject { described_class.new(url, name, version, resource) }
-    double(Resource, url: url, mirrors: [], specs: {})
-  end
-
-  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) }
+  let(:resource) { double(Resource, specs: {}) }
 
   describe "#initialize" do
     invalid_urls = %w[
@@ -278,10 +274,10 @@ describe ScpDownloadStrategy do
 
     invalid_urls.each do |invalid_url|
       context "with invalid URL #{invalid_url}" do
+        let(:url) { invalid_url }
+
         it "raises ScpDownloadStrategyError" do
-          expect {
+          expect { subject }.to raise_error(ScpDownloadStrategyError)
-            described_class.new(name, version, resource_for(invalid_url))
-          }.to raise_error(ScpDownloadStrategyError)
         end
       end
     end