Merge pull request #2725 from CocoaPods/lockfile-checkout-options

[Analyzer] Use specific checkout options from lockfile

CHANGELOG.md

@@ -14,6 +14,13 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
   [Samuel Giddins](https://github.com/segiddins)
   [#532](https://github.com/CocoaPods/CocoaPods/issues/532)
 
+* From now on, pods installed directly from their repositories will be recorded
+  in the `Podfile.lock` file and will be guaranteed to be checked-out using the
+  same revision on subsequent installations. Examples of this are when using
+  the `:git`, `:svn`, or `:hg` options in your `Podfile`.  
+  [Samuel Giddins](https://github.com/segiddins)
+  [#1058](https://github.com/CocoaPods/CocoaPods/issues/1058)
+
 ##### Bug Fixes
 
 * Fix an output formatting issue with various commands like `pod search`

Gemfile.lock

 GIT
   remote: https://github.com/CocoaPods/Core.git
-  revision: 60a486e8141323390dc5ff7f6676cf893a251a48
+  revision: 320bb0cf36d38f038a6478ad87618dd8bf148ffc
   branch: master
   specs:
     cocoapods-core (0.35.0.rc2)
@@ -69,7 +69,7 @@ PATH
 GEM
   remote: http://rubygems.org/
   specs:
-    activesupport (4.1.7)
+    activesupport (4.1.8)
       i18n (~> 0.6, >= 0.6.9)
       json (~> 1.7, >= 1.7.7)
       minitest (~> 5.1)

lib/cocoapods/external_sources.rb

@@ -13,9 +13,11 @@ module Pod
     #         hash.
     #
     def self.from_dependency(dependency, podfile_path)
-      name   = dependency.root_name
-      params = dependency.external_source
+      from_params(dependency.external_source, dependency, podfile_path)
+    end
 
+    def self.from_params(params, dependency, podfile_path)
+      name = dependency.root_name
       if klass = concrete_class_from_params(params)
         klass.new(name, params, podfile_path)
       else

lib/cocoapods/installer.rb

@@ -465,8 +465,9 @@ module Pod
     # @return [void]
     #
     def write_lockfiles
-      # checkout_options = sandbox.checkout_options
-      @lockfile = Lockfile.generate(podfile, analysis_result.specifications)
+      external_source_pods = podfile.dependencies.select(&:external_source).map(&:root_name).uniq
+      checkout_options = sandbox.checkout_sources.select { |root_name, _| external_source_pods.include? root_name }
+      @lockfile = Lockfile.generate(podfile, analysis_result.specifications, checkout_options)
 
       UI.message "- Writing Lockfile in #{UI.path config.lockfile_path}" do
         @lockfile.write_to_disk(config.lockfile_path)

lib/cocoapods/installer/analyzer.rb

@@ -53,6 +53,7 @@ module Pod
         @result.podfile_state = generate_podfile_state
         @locked_dependencies  = generate_version_locking_dependencies
 
+        store_existing_checkout_options
         fetch_external_sources if allow_fetches
         @result.specs_by_target = resolve_dependencies
         @result.specifications  = generate_specifications
@@ -264,33 +265,81 @@ module Pod
       #
       def fetch_external_sources
         return unless allow_pre_downloads?
-        deps_to_fetch = []
-        deps_to_fetch_if_needed = []
-        deps_with_external_source = podfile.dependencies.select(&:external_source)
 
-        deps_with_different_sources = podfile.dependencies.group_by(&:root_name).select { |_root_name, dependencies| dependencies.map(&:external_source).uniq.count > 1 }
+        verify_no_pods_with_different_sources!
+        unless dependencies_to_fetch.empty?
+          UI.section 'Fetching external sources' do
+            dependencies_to_fetch.sort.each do |dependency|
+              fetch_external_source(dependency, !pods_to_fetch.include?(dependency.name))
+            end
+          end
+        end
+      end
+
+      def verify_no_pods_with_different_sources!
+        deps_with_different_sources = podfile.dependencies.group_by(&:root_name).
+          select { |_root_name, dependencies| dependencies.map(&:external_source).uniq.count > 1 }
         deps_with_different_sources.each do |root_name, dependencies|
-          raise Informative, "There are multiple dependencies with different sources for `#{root_name}` in #{UI.path podfile.defined_in_file}:\n\n- #{dependencies.map(&:to_s).join("\n- ")}"
+          raise Informative, "There are multiple dependencies with different " \
+          "sources for `#{root_name}` in #{UI.path podfile.defined_in_file}:" \
+          "\n\n- #{dependencies.map(&:to_s).join("\n- ")}"
         end
+      end
+
+      def fetch_external_source(dependency, use_lockfile_options)
+        checkout_options = lockfile.checkout_options_for_pod_named(dependency.root_name) if lockfile
+        if checkout_options && use_lockfile_options
+          source = ExternalSources.from_params(checkout_options, dependency, podfile.defined_in_file)
+        else
+          source = ExternalSources.from_dependency(dependency, podfile.defined_in_file)
+        end
+        source.fetch(sandbox)
+      end
+
+      def dependencies_to_fetch
+        @deps_to_fetch ||= begin
+          deps_to_fetch = []
+          deps_to_fetch_if_needed = []
+          deps_with_external_source = podfile.dependencies.select(&:external_source)
 
           if update_mode == :all
             deps_to_fetch = deps_with_external_source
           else
+            deps_to_fetch = deps_with_external_source.select { |dep| pods_to_fetch.include?(dep.name) }
+            deps_to_fetch_if_needed = deps_with_external_source.select { |dep| result.podfile_state.unchanged.include?(dep.name) }
+            deps_to_fetch += deps_to_fetch_if_needed.select do |dep|
+              sandbox.specification(dep.name).nil? ||
+                !dep.external_source[:local].nil? ||
+                !dep.external_source[:path].nil? ||
+                !sandbox.pod_dir(dep.root_name).directory? ||
+                checkout_requires_update?(dep)
+            end
+          end
+          deps_to_fetch.uniq(&:root_name)
+        end
+      end
+
+      def checkout_requires_update?(dependency)
+        return true unless lockfile && sandbox.manifest
+        locked_checkout_options = lockfile.checkout_options_for_pod_named(dependency.root_name)
+        sandbox_checkout_options = sandbox.manifest.checkout_options_for_pod_named(dependency.root_name)
+        locked_checkout_options != sandbox_checkout_options
+      end
+
+      def pods_to_fetch
+        @pods_to_fetch ||= begin
           pods_to_fetch = result.podfile_state.added + result.podfile_state.changed
           if update_mode == :selected
             pods_to_fetch += update[:pods]
           end
-          deps_to_fetch = deps_with_external_source.select { |dep| pods_to_fetch.include?(dep.name) }
-          deps_to_fetch_if_needed = deps_with_external_source.select { |dep| result.podfile_state.unchanged.include?(dep.name) }
-          deps_to_fetch += deps_to_fetch_if_needed.select { |dep| sandbox.specification(dep.name).nil? || !dep.external_source[:local].nil? || !dep.external_source[:path].nil? || !sandbox.pod_dir(dep.name).directory? }
+          pods_to_fetch
         end
-
-        unless deps_to_fetch.empty?
-          UI.section 'Fetching external sources' do
-            deps_to_fetch.uniq(&:root_name).sort.each do |dependency|
-              source = ExternalSources.from_dependency(dependency, podfile.defined_in_file)
-              source.fetch(sandbox)
       end
+
+      def store_existing_checkout_options
+        podfile.dependencies.select(&:external_source).each do |dep|
+          if checkout_options = lockfile && lockfile.checkout_options_for_pod_named(dep.root_name)
+            sandbox.store_checkout_source(dep.root_name, checkout_options)
           end
         end
       end

lib/cocoapods/sandbox.rb

@@ -64,9 +64,13 @@ module Pod
     # @return [Lockfile] the manifest which contains the information about the
     #         installed pods.
     #
+    attr_accessor :manifest
+
     def manifest
+      @manifest ||= begin
         Lockfile.from_file(manifest_path) if manifest_path.exist?
       end
+    end
 
     # @return [Project] the Pods project.
     #

cocoapods-integration-specs @ 1b603f3f

-Subproject commit 365a0c8d93de662c019d6dbf3131f65842952357
+Subproject commit 1b603f3f9a53c497734ffaf9926416c0582064c2

spec/functional/command/project_spec.rb

@@ -63,7 +63,8 @@ module Pod
             s.version = '1.0'
           end,
         ]
-        Lockfile.generate(podfile, specs).write_to_disk(temporary_directory + 'Podfile.lock')
+        external_sources = {}
+        Lockfile.generate(podfile, specs, external_sources).write_to_disk(temporary_directory + 'Podfile.lock')
       end
 
       it 'for a single missing Pod' do

spec/integration.rb

@@ -176,6 +176,11 @@ describe_cli 'pod' do
       behaves_like cli_spec 'install_podfile_callbacks',
                             'install --no-repo-update'
     end
+
+    describe 'Uses Lockfile checkout options' do
+      behaves_like cli_spec 'install_using_checkout_options',
+                            'install --no-repo-update'
+    end
   end
 
   #--------------------------------------#

spec/spec_helper.rb

@@ -26,7 +26,9 @@ require 'bacon'
 require 'mocha-on-bacon'
 require 'pretty_bacon'
 require 'pathname'
+
 require 'active_support/core_ext/string/strip'
+require 'active_support/core_ext/object/deep_dup'
 
 ROOT = Pathname.new(File.expand_path('../../', __FILE__))
 $:.unshift((ROOT + 'lib').to_s)

spec/unit/installer/analyzer_spec.rb

 require File.expand_path('../../../spec_helper', __FILE__)
 
-# @return [Analyzer] the sample analyzer.
-#
-def create_analyzer
+#-----------------------------------------------------------------------------#
+
+module Pod
+  describe Installer::Analyzer do
+
+    describe 'Analysis' do
+      before do
         @podfile = Pod::Podfile.new do
           platform :ios, '6.0'
           xcodeproj 'SampleProject/SampleProject'
@@ -17,23 +21,12 @@ def create_analyzer
         hash['DEPENDENCIES'] = %w(JSONKit NUI SVPullToRefresh)
         hash['SPEC CHECKSUMS'] = {}
         hash['COCOAPODS'] = Pod::VERSION
-  lockfile = Pod::Lockfile.new(hash)
+        @lockfile = Pod::Lockfile.new(hash)
 
         SpecHelper.create_sample_app_copy_from_fixture('SampleProject')
-  analyzer = Pod::Installer::Analyzer.new(config.sandbox, @podfile, lockfile)
-end
-
-#-----------------------------------------------------------------------------#
-
-module Pod
-  describe Installer::Analyzer do
-
-    before do
-      @analyzer = create_analyzer
+        @analyzer = Pod::Installer::Analyzer.new(config.sandbox, @podfile, @lockfile)
       end
 
-    describe 'Analysis' do
-
       it 'returns whether an installation should be performed' do
         @analyzer.needs_install?.should.be.true
       end
@@ -185,57 +178,6 @@ module Pod
         @analyzer.send(:fetch_external_sources)
       end
 
-      it 'raises when dependencies with the same name have different ' \
-        'external sources' do
-        podfile = Podfile.new do
-          source 'https://github.com/CocoaPods/Specs.git'
-          xcodeproj 'SampleProject/SampleProject'
-          platform :ios
-          pod 'SEGModules', :git => 'https://github.com/segiddins/SEGModules.git'
-          pod 'SEGModules', :git => 'https://github.com/segiddins/Modules.git'
-        end
-        analyzer = Pod::Installer::Analyzer.new(config.sandbox, podfile, nil)
-        e = should.raise(Informative) { analyzer.analyze }
-
-        e.message.should.match /different sources for `SEGModules`/
-        e.message.should.match %r{SEGModules \(from `https://github.com/segiddins/SEGModules.git`\)}
-        e.message.should.match %r{SEGModules \(from `https://github.com/segiddins/Modules.git`\)}
-      end
-
-      it 'raises when dependencies with the same root name have different ' \
-        'external sources' do
-        podfile = Podfile.new do
-          source 'https://github.com/CocoaPods/Specs.git'
-          xcodeproj 'SampleProject/SampleProject'
-          platform :ios
-          pod 'RestKit/Core', :git => 'https://github.com/RestKit/RestKit.git'
-          pod 'RestKit', :git => 'https://github.com/segiddins/RestKit.git'
-        end
-        analyzer = Pod::Installer::Analyzer.new(config.sandbox, podfile, nil)
-        e = should.raise(Informative) { analyzer.analyze }
-
-        e.message.should.match /different sources for `RestKit`/
-        e.message.should.match %r{RestKit/Core \(from `https://github.com/RestKit/RestKit.git`\)}
-        e.message.should.match %r{RestKit \(from `https://github.com/segiddins/RestKit.git`\)}
-      end
-
-      it 'raises when dependencies with the same name have different ' \
-        'external sources with one being nil' do
-        podfile = Podfile.new do
-          source 'https://github.com/CocoaPods/Specs.git'
-          xcodeproj 'SampleProject/SampleProject'
-          platform :ios
-          pod 'RestKit', :git => 'https://github.com/RestKit/RestKit.git'
-          pod 'RestKit', '~> 0.23.0'
-        end
-        analyzer = Pod::Installer::Analyzer.new(config.sandbox, podfile, nil)
-        e = should.raise(Informative) { analyzer.analyze }
-
-        e.message.should.match /different sources for `RestKit`/
-        e.message.should.match %r{RestKit \(from `https://github.com/RestKit/RestKit.git`\)}
-        e.message.should.match %r{RestKit \(~> 0.23.0\)}
-      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::DownloaderSource.any_instance.expects(:specification_from_external).returns(Specification.new).once
@@ -311,8 +253,6 @@ module Pod
         state.added.sort.should == %w(AFNetworking JSONKit SVPullToRefresh libextobjc)
       end
 
-    end
-
       #-------------------------------------------------------------------------#
 
       describe 'Private helpers' do
@@ -609,4 +549,145 @@ module Pod
         end
       end
     end
+
+    describe 'Analysis, concerning naming' do
+      before do
+        SpecHelper.create_sample_app_copy_from_fixture('SampleProject')
+      end
+
+      it 'raises when dependencies with the same name have different ' \
+        'external sources' do
+        podfile = Podfile.new do
+          source 'https://github.com/CocoaPods/Specs.git'
+          xcodeproj 'SampleProject/SampleProject'
+          platform :ios
+          pod 'SEGModules', :git => 'https://github.com/segiddins/SEGModules.git'
+          pod 'SEGModules', :git => 'https://github.com/segiddins/Modules.git'
+        end
+        analyzer = Pod::Installer::Analyzer.new(config.sandbox, podfile, nil)
+        e = should.raise(Informative) { analyzer.analyze }
+
+        e.message.should.match /different sources for `SEGModules`/
+        e.message.should.match %r{SEGModules \(from `https://github.com/segiddins/SEGModules.git`\)}
+        e.message.should.match %r{SEGModules \(from `https://github.com/segiddins/Modules.git`\)}
+      end
+
+      it 'raises when dependencies with the same root name have different ' \
+        'external sources' do
+        podfile = Podfile.new do
+          source 'https://github.com/CocoaPods/Specs.git'
+          xcodeproj 'SampleProject/SampleProject'
+          platform :ios
+          pod 'RestKit/Core', :git => 'https://github.com/RestKit/RestKit.git'
+          pod 'RestKit', :git => 'https://github.com/segiddins/RestKit.git'
+        end
+        analyzer = Pod::Installer::Analyzer.new(config.sandbox, podfile, nil)
+        e = should.raise(Informative) { analyzer.analyze }
+
+        e.message.should.match /different sources for `RestKit`/
+        e.message.should.match %r{RestKit/Core \(from `https://github.com/RestKit/RestKit.git`\)}
+        e.message.should.match %r{RestKit \(from `https://github.com/segiddins/RestKit.git`\)}
+      end
+
+      it 'raises when dependencies with the same name have different ' \
+        'external sources with one being nil' do
+        podfile = Podfile.new do
+          source 'https://github.com/CocoaPods/Specs.git'
+          xcodeproj 'SampleProject/SampleProject'
+          platform :ios
+          pod 'RestKit', :git => 'https://github.com/RestKit/RestKit.git'
+          pod 'RestKit', '~> 0.23.0'
+        end
+        analyzer = Pod::Installer::Analyzer.new(config.sandbox, podfile, nil)
+        e = should.raise(Informative) { analyzer.analyze }
+
+        e.message.should.match /different sources for `RestKit`/
+        e.message.should.match %r{RestKit \(from `https://github.com/RestKit/RestKit.git`\)}
+        e.message.should.match %r{RestKit \(~> 0.23.0\)}
+      end
+    end
+
+    describe 'using lockfile checkout options' do
+      before do
+        @podfile = Pod::Podfile.new do
+          pod 'BananaLib', :git => 'example.com'
+        end
+        @dependency = @podfile.dependencies.first
+
+        @lockfile_checkout_options = { :git => 'example.com', :commit => 'commit' }
+        hash = {}
+        hash['PODS'] = ['BananaLib (1.0.0)']
+        hash['CHECKOUT OPTIONS'] = { 'BananaLib' => @lockfile_checkout_options }
+        hash['SPEC CHECKSUMS'] = {}
+        hash['COCOAPODS'] = Pod::VERSION
+        @lockfile = Pod::Lockfile.new(hash)
+
+        @analyzer = Pod::Installer::Analyzer.new(config.sandbox, @podfile, @lockfile)
+      end
+
+      it 'returns that an update is required when there is no sandbox manifest' do
+        @analyzer.sandbox.stubs(:manifest).returns(nil)
+        @analyzer.should.send(:checkout_requires_update?, @dependency)
+      end
+
+      before do
+        @sandbox_manifest = Pod::Lockfile.new(@lockfile.internal_data.deep_dup)
+        @analyzer.sandbox.manifest = @sandbox_manifest
+        @analyzer.sandbox.stubs(:specification).with('BananaLib').returns(stub)
+        pod_dir = stub
+        pod_dir.stubs(:directory?).returns(true)
+        @analyzer.sandbox.stubs(:pod_dir).with('BananaLib').returns(pod_dir)
+      end
+
+      it 'returns whether or not an update is required' do
+        @analyzer.send(:checkout_requires_update?, @dependency).should == false
+        @sandbox_manifest.send(:checkout_options_data).delete('BananaLib')
+        @analyzer.send(:checkout_requires_update?, @dependency).should == true
+      end
+
+      before do
+        @analyzer.result = Installer::Analyzer::AnalysisResult.new
+        @analyzer.result.podfile_state = Installer::Analyzer::SpecsState.new
+      end
+
+      it 'uses lockfile checkout options when no source exists in the sandbox' do
+        @analyzer.result.podfile_state.unchanged << 'BananaLib'
+        @sandbox_manifest.send(:checkout_options_data).delete('BananaLib')
+
+        downloader = stub('DownloaderSource')
+        ExternalSources.stubs(:from_params).with(@lockfile_checkout_options, @dependency, @podfile.defined_in_file).returns(downloader)
+
+        downloader.expects(:fetch)
+        @analyzer.send(:fetch_external_sources)
+      end
+
+      it 'uses lockfile checkout options when a different checkout exists in the sandbox' do
+        @analyzer.result.podfile_state.unchanged << 'BananaLib'
+        @sandbox_manifest.send(:checkout_options_data)['BananaLib'] = @lockfile_checkout_options.merge(:commit => 'other commit')
+
+        downloader = stub('DownloaderSource')
+        ExternalSources.stubs(:from_params).with(@lockfile_checkout_options, @dependency, @podfile.defined_in_file).returns(downloader)
+
+        downloader.expects(:fetch)
+        @analyzer.send(:fetch_external_sources)
+      end
+
+      it 'ignores lockfile checkout options when the podfile state has changed' do
+        @analyzer.result.podfile_state.changed << 'BananaLib'
+
+        downloader = stub('DownloaderSource')
+        ExternalSources.stubs(:from_params).with(@dependency.external_source, @dependency, @podfile.defined_in_file).returns(downloader)
+
+        downloader.expects(:fetch)
+        @analyzer.send(:fetch_external_sources)
+      end
+
+      it 'does not re-fetch the external source when the sandbox has the correct revision of the source' do
+        @analyzer.result.podfile_state.unchanged << 'BananaLib'
+
+        @analyzer.expects(:fetch_external_source).never
+        @analyzer.send(:fetch_external_sources)
+      end
+    end
+  end
 end