[ExternalSources] Refactor

CHANGELOG.md

@@ -43,6 +43,10 @@ To install or update CocoaPods see this [guide](http://docs.cocoapods.org/guides
   [Andrea Mazzini](https://github.com/andreamazz)
   [#2008](https://github.com/CocoaPods/CocoaPods/issues/2008)
 
+* Dependencies declared with external sources now support HTTP downloads and
+  have improved support for all the options supported by the downloader.  
+  [Fabio Pelosin][irrationalfab]
+
 ##### Bug Fixes
 
 * Support HTTP redirects when linting homepage and screenshots.

Gemfile.lock

@@ -7,7 +7,7 @@ GIT
 
 GIT
   remote: https://github.com/CocoaPods/Core.git
-  revision: f8a920b02c19273f689c1f930af580d473c7852f
+  revision: 46411b865c33a0f7fc172be70559611e2f7fbe21
   branch: master
   specs:
     cocoapods-core (0.31.1)
@@ -27,7 +27,7 @@ GIT
 
 GIT
   remote: https://github.com/CocoaPods/cocoapods-downloader.git
-  revision: d1cca92979882fe7dfef27001705ed4b9bc43faa
+  revision: a66c45ed7118540a2ff98ff30e916f9652d9eea3
   branch: master
   specs:
     cocoapods-downloader (0.4.1)

lib/cocoapods/external_sources.rb

@@ -13,20 +13,7 @@ module Pod
       name   = dependency.root_name
       params = dependency.external_source
 
-      klass  =  if params.key?(:git)          then GitSource
-                elsif params.key?(:svn)       then SvnSource
-                elsif params.key?(:hg)        then MercurialSource
-                elsif params.key?(:bzr)       then BazaarSource                  
-                elsif params.key?(:podspec)   then PodspecSource
-                elsif params.key?(:path)      then PathSource
-                end
-
-      if params.key?(:local)
-        klass = PathSource
-        UI.warn "The `:local` option of the Podfile has been renamed to `:path` and is deprecated." \
-      end
-
-      if klass
+      if klass = concrete_class_from_params(params)
         klass.new(name, params, podfile_path)
       else
         msg = "Unknown external source parameters for `#{name}`: `#{params}`"
@@ -34,6 +21,22 @@ module Pod
       end
     end
 
+    # @return [Class]
+    #
+    def self.concrete_class_from_params(params)
+      if params.key?(:podspec)
+        PodspecSource
+      elsif params.key?(:path)
+        PathSource
+      elsif params.key?(:local)
+        UI.warn "The `:local` option of the Podfile has been " \
+          "renamed to `:path` and it is deprecated."
+        PathSource
+      elsif Downloader.strategy_from_options(params)
+        DownloaderSource
+      end
+    end
+
     #-------------------------------------------------------------------------#
 
     # Abstract class that defines the common behaviour of external sources.
@@ -134,7 +137,8 @@ module Pod
       # @return [void]
       #
       def pre_download(sandbox)
-        UI.titled_section("Pre-downloading: `#{name}` #{description}", { :verbose_prefix => "-> " }) do
+        title = "Pre-downloading: `#{name}` #{description}"
+        UI.titled_section(title, { :verbose_prefix => "-> " }) do
           target = sandbox.root + name
           target.rmtree if target.exist?
           downloader = Config.instance.downloader(target, params)
@@ -175,103 +179,13 @@ module Pod
 
     #-------------------------------------------------------------------------#
 
-    # Provides support for fetching a specification file from a Git remote.
-    #
-    # Supports all the options of the downloader (is similar to the git key of
-    # `source` attribute of a specification).
-    #
-    # @note The podspec must be in the root of the repository and should have a
-    #       name matching the one of the dependency.
-    #
-    class GitSource < AbstractExternalSource
-
-      # @see AbstractExternalSource#fetch
-      #
-      def fetch(sandbox)
-        pre_download(sandbox)
-      end
-
-      # @see AbstractExternalSource#description
-      #
-      def description
-        "from `#{params[:git]}`".tap do |description|
-          description << ", commit `#{params[:commit]}`" if params[:commit]
-          description << ", branch `#{params[:branch]}`" if params[:branch]
-          description << ", tag `#{params[:tag]}`" if params[:tag]
-        end
-      end
-    end
-
-    #-------------------------------------------------------------------------#
-
-    # Provides support for fetching a specification file from a SVN source
-    # remote.
-    #
-    # Supports all the options of the downloader (is similar to the git key of
-    # `source` attribute of a specification).
-    #
-    # @note The podspec must be in the root of the repository and should have a
-    #       name matching the one of the dependency.
-    #
-    class SvnSource < AbstractExternalSource
-
-      # @see AbstractExternalSource#fetch
-      #
-      def fetch(sandbox)
-        pre_download(sandbox)
-      end
-
-      # @see AbstractExternalSource#description
-      #
-      def description
-        "from `#{params[:svn]}`".tap do |description|
-          description << ", folder `#{params[:folder]}`" if params[:folder]
-          description << ", tag `#{params[:tag]}`" if params[:tag]
-          description << ", revision `#{params[:revision]}`" if params[:revision]
-        end
-      end
-    end
-
-    #-------------------------------------------------------------------------#
-
-    # Provides support for fetching a specification file from a Mercurial
-    # source remote.
-    #
-    # Supports all the options of the downloader (is similar to the git key of
-    # `source` attribute of a specification).
-    #
-    # @note The podspec must be in the root of the repository and should have a
-    #       name matching the one of the dependency.
-    #
-    class MercurialSource < AbstractExternalSource
-
-      # @see AbstractExternalSource#fetch
-      #
-      def fetch(sandbox)
-        pre_download(sandbox)
-      end
-
-      # @see AbstractExternalSource#description
-      #
-      def description
-        "from `#{params[:hg]}`".tap do |description|
-          description << ", revision `#{params[:revision]}`" if params[:revision]
-        end
-      end
-    end
-
-    #-------------------------------------------------------------------------#
-
-    # Provides support for fetching a specification file from a Bazaar
-    # source remote.
-    #
-    # Supports all the options of the downloader (is similar to the git key of
-    # `source` attribute of a specification).
+    # Provides support for fetching a specification file from a source handled
+    # by the downloader. Supports all the options of the downloader
     #
     # @note The podspec must be in the root of the repository and should have a
     #       name matching the one of the dependency.
     #
-    class BazaarSource < AbstractExternalSource
+    class DownloaderSource < AbstractExternalSource
 
       # @see AbstractExternalSource#fetch
       #
@@ -282,10 +196,14 @@ module Pod
       # @see AbstractExternalSource#description
       #
       def description
-        "from `#{params[:bzr]}`".tap do |description|
-          description << ", tag `#{params[:tag]}`" if params[:tag]
-          description << ", revision `#{params[:revision]}`" if params[:revision]
+        strategy = Downloader.strategy_from_options(params)
+        options = params.dup
+        url = options.delete(strategy)
+        result = "from `#{url}`"
+        options.each do |key, value|
+          result << ", #{key} `#{value}`"
         end
+        result
       end
     end
 
@@ -300,7 +218,6 @@ module Pod
       #
       def fetch(sandbox)
         UI.titled_section("Fetching podspec for `#{name}` #{description}", { :verbose_prefix => "-> " }) do
-
           require 'open-uri'
           open(podspec_uri) { |io| store_podspec(sandbox, io.read) }
         end
@@ -374,7 +291,7 @@ module Pod
       def declared_path
         Pathname.new params[:path] || params[:local]
       end
-      
+
       # @return [Pathname] the path of the podspec.
       #
       def podspec_path

spec/unit/external_sources_spec.rb

 require File.expand_path('../../spec_helper', __FILE__)
 
 module Pod
-
   describe ExternalSources do
-    it "returns the instance of appropriate concrete class according to the parameters" do
-      git     = Dependency.new("Reachability", :git     => nil)
-      svn     = Dependency.new("Reachability", :svn     => nil)
-      podspec = Dependency.new("Reachability", :podspec => nil)
-      local   = Dependency.new("Reachability", :local   => nil)
-      path    = Dependency.new("Reachability", :path   => nil)
-
-      ExternalSources.from_dependency(git, nil).class.should     == ExternalSources::GitSource
-      ExternalSources.from_dependency(svn, nil).class.should     == ExternalSources::SvnSource
-      ExternalSources.from_dependency(podspec, nil).class.should == ExternalSources::PodspecSource
-      ExternalSources.from_dependency(local, nil).class.should   == ExternalSources::PathSource
-      ExternalSources.from_dependency(path, nil).class.should    == ExternalSources::PathSource
+    before do
+      @subject = ExternalSources
+    end
+
+    describe "from_dependency" do
+      it "supports a podspec source" do
+        dep = Dependency.new("Reachability", :podspec => nil)
+        klass = @subject.from_dependency(dep, nil).class
+        klass.should == @subject::PodspecSource
+      end
+
+      it "supports a path source" do
+        dep = Dependency.new("Reachability", :path => nil)
+        klass = @subject.from_dependency(dep, nil).class
+        klass.should == @subject::PathSource
+      end
+
+      it "supports a path source specified with the legacy :local key" do
+        dep = Dependency.new("Reachability", :local => nil)
+        klass = @subject.from_dependency(dep, nil).class
+        klass.should == @subject::PathSource
+      end
+
+      it "supports all the strategies implemented by the downloader" do
+        [:git, :svn, :hg, :bzr, :http].each do |strategy|
+          dep     = Dependency.new("Reachability", strategy => nil)
+          klass = @subject.from_dependency(dep, nil).class
+          klass.should == @subject::DownloaderSource
+        end
+      end
     end
   end
 
@@ -24,7 +41,7 @@ module Pod
 
     before do
       dependency = Dependency.new("Reachability", :git => fixture('integration/Reachability'))
-      @external_source = ExternalSources.from_dependency(dependency, nil)
+      @subject = ExternalSources.from_dependency(dependency, nil)
     end
 
     #--------------------------------------#
@@ -43,7 +60,7 @@ module Pod
 
       it "fetches the specification from the remote stores it in the sandbox" do
         config.sandbox.specification('Reachability').should == nil
-        @external_source.fetch(config.sandbox)
+        @subject.fetch(config.sandbox)
         config.sandbox.specification('Reachability').name.should == 'Reachability'
       end
 
@@ -55,7 +72,7 @@ module Pod
 
       it "pre-downloads the Pod and stores the relevant information in the sandbox" do
         sandbox = config.sandbox
-        @external_source.send(:pre_download, sandbox)
+        @subject.send(:pre_download, sandbox)
         path = config.sandbox.root + 'Local Podspecs/Reachability.podspec'
         path.should.exist?
         sandbox.predownloaded_pods.should == ["Reachability"]
@@ -73,101 +90,31 @@ module Pod
 
   #---------------------------------------------------------------------------#
 
-  describe ExternalSources::GitSource do
+  describe ExternalSources::DownloaderSource do
 
     before do
-      dependency = Dependency.new("Reachability", :git => fixture('integration/Reachability'))
-      @external_source = ExternalSources.from_dependency(dependency, nil)
+      params = {
+        :git => fixture('integration/Reachability'),
+        :branch => 'master'
+      }
+      dep = Dependency.new("Reachability", params)
+      @subject = ExternalSources.from_dependency(dep, nil)
     end
 
     it "creates a copy of the podspec" do
-      @external_source.fetch(config.sandbox)
+      @subject.fetch(config.sandbox)
       path = config.sandbox.root + 'Local Podspecs/Reachability.podspec'
       path.should.exist?
     end
 
-    it "marks a LocalPod as downloaded" do
-      @external_source.fetch(config.sandbox)
+    it "marks the Pod as pre-downloaded" do
+      @subject.fetch(config.sandbox)
       config.sandbox.predownloaded_pods.should == ["Reachability"]
     end
 
     it "returns the description" do
-      @external_source.description.should.match %r|from `.*Reachability`|
-    end
-  end
-
-  #---------------------------------------------------------------------------#
-
-  describe ExternalSources::SvnSource do
-
-    before do
-      dependency = Dependency.new("SvnSource", :svn => "file://#{fixture('subversion-repo/trunk')}")
-      @external_source = ExternalSources.from_dependency(dependency, nil)
-    end
-
-    it "creates a copy of the podspec" do
-      @external_source.fetch(config.sandbox)
-      path = config.sandbox.root + 'Local Podspecs/SvnSource.podspec'
-      path.should.exist?
-    end
-
-    it "marks a LocalPod as downloaded" do
-      @external_source.fetch(config.sandbox)
-      config.sandbox.predownloaded_pods.should == ["SvnSource"]
-    end
-
-    it "returns the description" do
-      @external_source.description.should.match %r|from `.*subversion-repo/trunk`|
-    end
-  end
-
-  #---------------------------------------------------------------------------#
-
-  describe ExternalSources::MercurialSource do
-
-    before do
-      dependency = Dependency.new("MercurialSource", :hg => fixture('mercurial-repo'))
-      @external_source = ExternalSources.from_dependency(dependency, nil)
-    end
-
-    it "creates a copy of the podspec" do
-      @external_source.fetch(config.sandbox)
-      path = config.sandbox.root + 'Local Podspecs/MercurialSource.podspec'
-      path.should.exist?
-    end
-
-    it "marks a LocalPod as downloaded" do
-      @external_source.fetch(config.sandbox)
-      config.sandbox.predownloaded_pods.should == ["MercurialSource"]
-    end
-
-    it "returns the description" do
-      @external_source.description.should.match %r|from `.*/mercurial-repo`|
-    end
-  end
-
-  #---------------------------------------------------------------------------#
-
-  describe ExternalSources::BazaarSource do
-
-    before do
-      dependency = Dependency.new("BazaarSource", :bzr => fixture('bzr-repo'))
-      @external_source = ExternalSources.from_dependency(dependency, nil)
-    end
-
-    it "creates a copy of the podspec" do
-      @external_source.fetch(config.sandbox)
-      path = config.sandbox.root + 'Local Podspecs/BazaarSource.podspec'
-      path.should.exist?
-    end
-
-    it "marks a LocalPod as downloaded" do
-      @external_source.fetch(config.sandbox)
-      config.sandbox.predownloaded_pods.should == ["BazaarSource"]
-    end
-
-    it "returns the description" do
-      @external_source.description.should.match %r|from `.*/bzr-repo`|
+      expected = /from `.*Reachability`, branch `master`/
+      @subject.description.should.match(expected)
     end
   end
 
@@ -179,49 +126,49 @@ module Pod
       podspec_path = fixture('integration/Reachability/Reachability.podspec')
       dependency = Dependency.new("Reachability", :podspec => podspec_path.to_s)
       podfile_path = fixture('integration/Podfile')
-      @external_source = ExternalSources.from_dependency(dependency, podfile_path)
+      @subject = ExternalSources.from_dependency(dependency, podfile_path)
     end
 
     it "creates a copy of the podspec" do
-      @external_source.fetch(config.sandbox)
+      @subject.fetch(config.sandbox)
       path = config.sandbox.root + 'Local Podspecs/Reachability.podspec'
       path.should.exist?
     end
 
     it "returns the description" do
-      @external_source.description.should.match %r|from `.*Reachability/Reachability.podspec`|
+      @subject.description.should.match %r|from `.*Reachability/Reachability.podspec`|
     end
 
     describe "Helpers" do
 
       it "handles absolute paths" do
-        @external_source.stubs(:params).returns(:podspec => fixture('integration/Reachability'))
-        path = @external_source.send(:podspec_uri)
+        @subject.stubs(:params).returns(:podspec => fixture('integration/Reachability'))
+        path = @subject.send(:podspec_uri)
         path.should == fixture('integration/Reachability/Reachability.podspec').to_s
       end
 
       it "handles paths when there is no podfile path" do
-        @external_source.stubs(:podfile_path).returns(nil)
-        @external_source.stubs(:params).returns(:podspec => fixture('integration/Reachability'))
-        path = @external_source.send(:podspec_uri)
+        @subject.stubs(:podfile_path).returns(nil)
+        @subject.stubs(:params).returns(:podspec => fixture('integration/Reachability'))
+        path = @subject.send(:podspec_uri)
         path.should == fixture('integration/Reachability/Reachability.podspec').to_s
       end
 
       it "handles relative paths" do
-        @external_source.stubs(:params).returns(:podspec => 'Reachability')
-        path = @external_source.send(:podspec_uri)
+        @subject.stubs(:params).returns(:podspec => 'Reachability')
+        path = @subject.send(:podspec_uri)
         path.should == fixture('integration/Reachability/Reachability.podspec').to_s
       end
 
       it "expands the tilde" do
-        @external_source.stubs(:params).returns(:podspec => '~/Reachability')
-        path = @external_source.send(:podspec_uri)
+        @subject.stubs(:params).returns(:podspec => '~/Reachability')
+        path = @subject.send(:podspec_uri)
         path.should == ENV['HOME'] + '/Reachability/Reachability.podspec'
       end
 
-      it "handles urls" do
-        @external_source.stubs(:params).returns(:podspec => "http://www.example.com/Reachability.podspec")
-        path = @external_source.send(:podspec_uri)
+      it "handles URLs" do
+        @subject.stubs(:params).returns(:podspec => "http://www.example.com/Reachability.podspec")
+        path = @subject.send(:podspec_uri)
         path.should == "http://www.example.com/Reachability.podspec"
       end
     end
@@ -235,11 +182,11 @@ module Pod
       podspec_path = fixture('integration/Reachability/Reachability.podspec')
       dependency = Dependency.new("Reachability", :path => fixture('integration/Reachability'))
       podfile_path = fixture('integration/Podfile')
-      @external_source = ExternalSources.from_dependency(dependency, podfile_path)
+      @subject = ExternalSources.from_dependency(dependency, podfile_path)
     end
 
     it "creates a copy of the podspec" do
-      @external_source.fetch(config.sandbox)
+      @subject.fetch(config.sandbox)
       path = config.sandbox.root + 'Local Podspecs/Reachability.podspec'
       path.should.exist?
     end
@@ -254,11 +201,11 @@ module Pod
     end
 
     it "returns the description" do
-      @external_source.description.should.match %r|from `.*integration/Reachability`|
+      @subject.description.should.match %r|from `.*integration/Reachability`|
     end
 
     it "marks the Pod as local in the sandbox" do
-      @external_source.fetch(config.sandbox)
+      @subject.fetch(config.sandbox)
       config.sandbox.development_pods.should == {
         "Reachability" => fixture('integration/Reachability').to_s
       }
@@ -267,34 +214,34 @@ module Pod
     describe "Helpers" do
 
       it "handles absolute paths" do
-        @external_source.stubs(:params).returns(:path => fixture('integration/Reachability'))
-        path = @external_source.send(:podspec_path)
+        @subject.stubs(:params).returns(:path => fixture('integration/Reachability'))
+        path = @subject.send(:podspec_path)
         path.should == fixture('integration/Reachability/Reachability.podspec')
       end
 
       it "handles paths when there is no podfile path" do
-        @external_source.stubs(:podfile_path).returns(nil)
-        @external_source.stubs(:params).returns(:path => fixture('integration/Reachability'))
-        path = @external_source.send(:podspec_path)
+        @subject.stubs(:podfile_path).returns(nil)
+        @subject.stubs(:params).returns(:path => fixture('integration/Reachability'))
+        path = @subject.send(:podspec_path)
         path.should == fixture('integration/Reachability/Reachability.podspec')
       end
 
       it "handles relative paths" do
-        @external_source.stubs(:params).returns(:path => 'Reachability')
-        path = @external_source.send(:podspec_path)
+        @subject.stubs(:params).returns(:path => 'Reachability')
+        path = @subject.send(:podspec_path)
         path.should == fixture('integration/Reachability/Reachability.podspec')
       end
 
       it "expands the tilde" do
-        @external_source.stubs(:params).returns(:path => '~/Reachability')
+        @subject.stubs(:params).returns(:path => '~/Reachability')
         Pathname.any_instance.stubs(:exist?).returns(true)
-        path = @external_source.send(:podspec_path)
+        path = @subject.send(:podspec_path)
         path.should == Pathname(ENV['HOME']) + 'Reachability/Reachability.podspec'
       end
 
       it "raises if the podspec cannot be found" do
-        @external_source.stubs(:params).returns(:path => temporary_directory)
-        e = lambda { @external_source.send(:podspec_path) }.should.raise Informative
+        @subject.stubs(:params).returns(:path => temporary_directory)
+        e = lambda { @subject.send(:podspec_path) }.should.raise Informative
         e.message.should.match /No podspec found for `Reachability` in `#{temporary_directory}`/
       end
     end

spec/unit/installer/analyzer_spec.rb

@@ -129,19 +129,19 @@ module Pod
         podfile_state.added << "BananaLib"
         @analyzer.stubs(:result).returns(stub(:podfile_state => podfile_state))
         @podfile.stubs(:dependencies).returns([Dependency.new('BananaLib', :git => "example.com")])
-        ExternalSources::GitSource.any_instance.expects(:fetch)
+        ExternalSources::DownloaderSource.any_instance.expects(:fetch)
         @analyzer.send(:fetch_external_sources)
       end
 
       xit "it fetches the specification from either the sandbox or from the remote be default" do
         dependency = Dependency.new('Name', :git => 'www.example.com')
-        ExternalSources::GitSource.any_instance.expects(:specification_from_external).returns(Specification.new).once
+        ExternalSources::DownloaderSource.any_instance.expects(:specification_from_external).returns(Specification.new).once
         @resolver.send(:set_from_external_source, dependency)
       end
 
       xit "it fetches the specification from the remote if in update mode" do
         dependency = Dependency.new('Name', :git => 'www.example.com')
-        ExternalSources::GitSource.any_instance.expects(:specification).returns(Specification.new).once
+        ExternalSources::DownloaderSource.any_instance.expects(:specification).returns(Specification.new).once
         @resolver.update_external_specs = false
         @resolver.send(:set_from_external_source, dependency)
       end