Merge pull request #5014 from DanielTomlinson/dan-sources-manager-refactor

Refactor SourcesManager to use Source/MasterSource

CHANGELOG.md

@@ -8,7 +8,11 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
 
 ##### Enhancements
 
-* None.  
+* The master specs repo will no longer perform 'no-op' git fetches. This should
+  help to reduce the load on GitHub's servers.  
+  [Daniel Tomlinson](https://github.com/DanielTomlinson)
+  [#5005](https://github.com/CocoaPods/CocoaPods/issues/5005)
+  [#4989](https://github.com/CocoaPods/CocoaPods/issues/4989)
 
 ##### Bug Fixes
 

Gemfile.lock

@@ -7,7 +7,7 @@ GIT
 
 GIT
   remote: https://github.com/CocoaPods/Core.git
-  revision: 10e44cb334f8b556ce86fda8e25873fc6a8bddf3
+  revision: 66c7b3b40fde614bb3eb6e287c25da4dfc6529c1
   branch: master
   specs:
     cocoapods-core (1.0.0.beta.5)

lib/cocoapods/command/outdated.rb

@@ -91,7 +91,7 @@ module Pod
       def spec_sets
         @spec_sets ||= begin
           analyzer.send(:update_repositories) unless config.skip_repo_update?
-          aggregate = Source::Aggregate.new(analyzer.sources.map(&:repo))
+          aggregate = Source::Aggregate.new(analyzer.sources)
           installed_pods.map do |pod_name|
             aggregate.search(Dependency.new(pod_name))
           end.compact.uniq

lib/cocoapods/executable.rb

@@ -137,9 +137,9 @@ module Pod
       case capture
       when :merge then Open3.capture2e(bin, *command)
       when :both then Open3.capture3(bin, *command)
-      when :out then Open3.capture2(bin, *command)
+      when :out then Open3.capture3(bin, *command).values_at(0, -1)
       when :err then Open3.capture3(bin, *command).drop(1)
-      when :none then Open3.capture2(bin, *command).last
+      when :none then Open3.capture3(bin, *command).last
       end
     end
 

lib/cocoapods/resolver.rb

@@ -352,7 +352,7 @@ module Pod
       if dependency && dependency.podspec_repo
         return SourcesManager.aggregate_for_dependency(dependency)
       else
-        @aggregate ||= Source::Aggregate.new(sources.map(&:repo))
+        @aggregate ||= Source::Aggregate.new(sources)
       end
     end
 

lib/cocoapods/sources_manager.rb

@@ -41,7 +41,7 @@ module Pod
       #
       def sources(names)
         dirs = names.map { |name| source_dir(name) }
-        dirs.map { |repo| Source.new(repo) }
+        dirs.map { |repo| source_from_path(repo) }
       end
 
       # Returns the source whose {Source#url} is equal to `url`, adding the repo
@@ -97,7 +97,7 @@ module Pod
       def all
         return [] unless config.repos_dir.exist?
         dirs = config.repos_dir.children.select(&:directory?)
-        dirs.map { |repo| Source.new(repo) }
+        dirs.map { |repo| source_from_path(repo) }
       end
 
       # @return [Array<Source>] The CocoaPods Master Repo source.
@@ -323,7 +323,7 @@ module Pod
         changed_spec_paths = {}
         sources.each do |source|
           UI.section "Updating spec repo `#{source.name}`" do
-            changed_source_paths = source.update(show_output && !config.verbose?)
+            changed_source_paths = source.update(show_output)
             changed_spec_paths[source] = changed_source_paths if changed_source_paths.count > 0
             check_version_information(source.repo)
           end
@@ -468,14 +468,25 @@ module Pod
 
       private
 
+      # @return [Source] The Source at a given path.
+      #
+      # @param [Pathname] path
+      #        The local file path to one podspec repo.
+      #
+      def source_from_path(path)
+        return Source.new(path) unless path.basename.to_s == 'master'
+        MasterSource.new(path)
+      end
+
       # @return [Source::Aggregate] The aggregate of the sources from repos.
       #
       # @param  [Array<Pathname>] repos
       #         The local file paths to one or more podspec repo caches.
       #
       def aggregate_with_repos(repos)
+        sources = repos.map { |path| source_from_path(path) }
         @aggregates_by_repos ||= {}
-        @aggregates_by_repos[repos] ||= Source::Aggregate.new(repos)
+        @aggregates_by_repos[repos] ||= Source::Aggregate.new(sources)
       end
 
       # @return [Bool] Whether the given path is writable by the current user.
@@ -609,9 +620,14 @@ module Pod
     extend Executable
     executable :git
 
+    def git(args, include_error: false)
+      Executable.capture_command('git', args, :capture => include_error ? :merge : :out).first.strip
+    end
+
     def update_git_repo(show_output = false)
-      output = git! %w(pull --ff-only)
-      UI.puts output if show_output
+      Config.instance.with_changes(:verbose => show_output) do
+        git!(%w(pull --ff-only))
+      end
     rescue
       UI.warn 'CocoaPods was not able to update the ' \
                 "`#{name}` repo. If this is an unexpected issue " \

spec/functional/command/repo/update_spec.rb

 require File.expand_path('../../../../spec_helper', __FILE__)
+require 'webmock'
 
 module Pod
   describe Command::Repo::Update do
@@ -8,6 +9,15 @@ module Pod
     before do
       set_up_test_repo
       config.repos_dir = SpecHelper.tmp_repos_path
+
+      MasterSource.any_instance.stubs(:git_commit_hash).returns('commit hash')
+      WebMock.stub_request(:get, 'https://api.github.com/repos/CocoaPods/Specs/commits/master').
+        with(:headers => { 'Accept' => 'application/vnd.github.chitauri-preview+sha', 'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', 'If-None-Match' => '"commit hash"', 'User-Agent' => 'CocoaPods' }).
+        to_return(:status => 200, :body => '', :headers => {})
+    end
+
+    after do
+      WebMock.reset!
     end
 
     it 'updates a repository' do

spec/unit/installer/analyzer_spec.rb

@@ -4,7 +4,7 @@ module Pod
   describe Installer::Analyzer do
     describe 'Analysis' do
       before do
-        repos = [fixture('spec-repos/test_repo'), fixture('spec-repos/master')]
+        repos = [Source.new(fixture('spec-repos/test_repo')), MasterSource.new(fixture('spec-repos/master'))]
         aggregate = Pod::Source::Aggregate.new(repos)
         Pod::SourcesManager.stubs(:aggregate).returns(aggregate)
         aggregate.sources.first.stubs(:url).returns(SpecHelper.test_repo_url)
@@ -323,7 +323,7 @@ module Pod
 
       describe 'no-integrate platform validation' do
         before do
-          repos = [fixture('spec-repos/test_repo')]
+          repos = [Source.new(fixture('spec-repos/test_repo'))]
           aggregate = Pod::Source::Aggregate.new(repos)
           Pod::SourcesManager.stubs(:aggregate).returns(aggregate)
           aggregate.sources.first.stubs(:url).returns(SpecHelper.test_repo_url)

spec/unit/sources_manager_spec.rb

 require File.expand_path('../../spec_helper', __FILE__)
+require 'webmock'
 
 def set_up_test_repo_for_update
   set_up_test_repo
@@ -260,8 +261,8 @@ module Pod
 
           it 'runs `pod repo add` when there is no matching source' do
             Command::Repo::Add.any_instance.stubs(:run).once
-            SourcesManager.stubs(:source_with_url).returns(nil).then.returns('Source')
-            SourcesManager.find_or_create_source_with_url('https://github.com/artsy/Specs.git').
+            SourcesManager.stubs(:source_with_url).returns(nil).then.returns(Source.new('Source'))
+            SourcesManager.find_or_create_source_with_url('https://github.com/artsy/Specs.git').name.
               should == 'Source'
           end
 
@@ -295,26 +296,35 @@ module Pod
     describe 'Updating Sources' do
       extend SpecHelper::TemporaryRepos
 
+      before do
+        MasterSource.any_instance.stubs(:requires_update?).returns(true)
+      end
+
       it 'updates source backed by a git repository' do
         set_up_test_repo_for_update
         SourcesManager.expects(:update_search_index_if_needed_in_background).with({}).returns(nil)
+        MasterSource.any_instance.expects(:git!).with(%w(pull --ff-only))
         SourcesManager.update(test_repo_path.basename.to_s, true)
-        UI.output.should.match /is up to date/
       end
 
       it 'uses the only fast forward git option' do
         set_up_test_repo_for_update
-        Source.any_instance.expects(:git!).with { |options| options.should.include? '--ff-only' }
+        MasterSource.any_instance.expects(:git!).with { |options| options.should.include? '--ff-only' }
         SourcesManager.expects(:update_search_index_if_needed_in_background).with({}).returns(nil)
         SourcesManager.update(test_repo_path.basename.to_s, true)
       end
 
       it 'prints a warning if the update failed' do
-        UI.warnings = ''
         set_up_test_repo_for_update
-        Dir.chdir(test_repo_path) do
-          `git remote set-url origin file:///dev/null`
-        end
+        Source.any_instance.stubs(:git).with(%w(rev-parse HEAD)).returns('aabbccd')
+        Source.any_instance.stubs(:git).with(%w(diff --name-only aabbccd..HEAD)).returns('')
+        MasterSource.any_instance.expects(:git!).with(%w(pull --ff-only)).raises(<<-EOS)
+fatal: '/dev/null' does not appear to be a git repository
+fatal: Could not read from remote repository.
+
+Please make sure you have the correct access rights
+and the repository exists.
+        EOS
         SourcesManager.expects(:update_search_index_if_needed_in_background).with({}).returns(nil)
         SourcesManager.update(test_repo_path.basename.to_s, true)
         UI.warnings.should.include('not able to update the `master` repo')

spec/unit/validator_spec.rb

@@ -418,6 +418,7 @@ module Pod
         validator.stubs(:validate_url)
         git = Executable.which(:git)
         Executable.stubs(:which).with('git').returns(git)
+        Executable.stubs(:capture_command).with('git', ['config', '--get', 'remote.origin.url'], :capture => :out).returns(['https://github.com/CocoaPods/Specs.git'])
         Executable.stubs(:which).with(:xcrun)
         Executable.expects(:which).with('xcodebuild').times(4).returns('/usr/bin/xcodebuild')
         command = %w(clean build -workspace App.xcworkspace -scheme App -configuration Release)