Merge pull request #2748 from CocoaPods/seg-duplicate-dependencies

[Analyzer] Improve handling of different types of duplicate dependencies

CHANGELOG.md

@@ -67,6 +67,18 @@ To install or update CocoaPods see this [guide](http://docs.cocoapods.org/guides
   [Eloy Durán](https://github.com/alloy)
   [#2722](https://github.com/CocoaPods/CocoaPods/pull/2722)
 
+* Analysis is now halted and the user informed when there are multiple different
+  external sources for dependencies with the same root name.
+  The user is also now warned when there are duplicate dependencies in the
+  Podfile.  
+  [Samuel Giddins](https://github.com/segiddins)
+  [#2738](https://github.com/CocoaPods/CocoaPods/issues/2738)
+
+* Multiple subspecs that point to the same external dependency will now only
+  cause that external source to be fetched once.  
+  [Samuel Giddins](https://github.com/segiddins)
+  [#2743](https://github.com/CocoaPods/CocoaPods/issues/2743)
+
 ##### Bug Fixes
 
 * Do not try to clone spec-repos in `/`.  

lib/cocoapods/installer/analyzer.rb

@@ -268,6 +268,11 @@ module Pod
         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 }
+        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- ")}"
+        end
+
         if update_mode == :all
           deps_to_fetch = deps_with_external_source
         else
@@ -282,7 +287,7 @@ module Pod
 
         unless deps_to_fetch.empty?
           UI.section 'Fetching external sources' do
-            deps_to_fetch.uniq.sort.each do |dependency|
+            deps_to_fetch.uniq(&:root_name).sort.each do |dependency|
               source = ExternalSources.from_dependency(dependency, podfile.defined_in_file)
               source.fetch(sandbox)
             end
@@ -310,6 +315,13 @@ module Pod
       #         grouped by target.
       #
       def resolve_dependencies
+        duplicate_dependencies = podfile.dependencies.group_by(&:name).
+          select { |_name, dependencies| dependencies.count > 1 }
+        duplicate_dependencies.each do |name, dependencies|
+          UI.warn "There are duplicate dependencies on `#{name}` in #{UI.path podfile.defined_in_file}:\n\n" \
+           "- #{dependencies.map(&:to_s).join("\n- ")}"
+        end
+
         specs_by_target = nil
         UI.section "Resolving dependencies of #{UI.path podfile.defined_in_file}" do
           resolver = Resolver.new(sandbox, podfile, locked_dependencies, sources)

spec/spec_helper.rb

@@ -82,6 +82,16 @@ module Bacon
     include SpecHelper::Fixture
     include SpecHelper::Command
 
+    alias_method :original_initialize, :initialize
+    def initialize(name, &block)
+      original_initialize(name, &block)
+      before do
+        Pod::UI.output = ''
+        Pod::UI.warnings = ''
+        Pod::UI.next_input = ''
+      end
+    end
+
     def skip_xcodebuild?
       ENV['SKIP_XCODEBUILD']
     end

spec/unit/installer/analyzer_spec.rb

@@ -173,6 +173,69 @@ module Pod
         @analyzer.send(:fetch_external_sources)
       end
 
+      it 'does not download the same source multiple times for different subspecs' do
+        podfile_state = Installer::Analyzer::SpecsState.new
+        podfile_state.added << 'ARAnalytics/Mixpanel' << 'ARAnalytics/HockeyApp'
+        @analyzer.stubs(:result).returns(stub(:podfile_state => podfile_state))
+        @podfile.stubs(:dependencies).returns([
+          Dependency.new('ARAnalytics/Mixpanel', :git => 'https://github.com/orta/ARAnalytics', :commit => '6f1a1c314894437e7e5c09572c276e644dbfb64b'),
+          Dependency.new('ARAnalytics/HockeyApp', :git => 'https://github.com/orta/ARAnalytics', :commit => '6f1a1c314894437e7e5c09572c276e644dbfb64b'),
+        ])
+        ExternalSources::DownloaderSource.any_instance.expects(:fetch).once
+        @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
@@ -224,6 +287,24 @@ module Pod
 
       #--------------------------------------#
 
+      it 'warns when a dependency is duplicated' do
+        podfile = Podfile.new do
+          source 'https://github.com/CocoaPods/Specs.git'
+          xcodeproj 'SampleProject/SampleProject'
+          platform :ios, '8.0'
+          pod 'RestKit', '~> 0.23.0'
+          pod 'RestKit', '<= 0.23.2'
+        end
+        analyzer = Pod::Installer::Analyzer.new(config.sandbox, podfile, nil)
+        analyzer.analyze
+
+        UI.warnings.should.match /duplicate dependencies on `RestKit`/
+        UI.warnings.should.match %r{RestKit \(~> 0.23.0\)}
+        UI.warnings.should.match %r{RestKit \(<= 0.23.2\)}
+      end
+
+      #--------------------------------------#
+
       it 'computes the state of the Sandbox respect to the resolved dependencies' do
         @analyzer.stubs(:lockfile).returns(nil)
         state = @analyzer.analyze.sandbox_state