[Analyzer] Fix transitive dependency selection

lib/cocoapods/installer/analyzer.rb

@@ -305,48 +305,68 @@ module Pod
               generate_pod_target(target_definitions, variant.specs, :scope_suffix => suffixes[variant])
             end
           end
+
+          all_specs = specs_by_target.values.flatten.uniq
+          pod_targets_by_name = pod_targets.group_by(&:pod_name).each_with_object({}) do |(name, values), hash|
+            # Sort the target by the number of activated subspecs, so that
+            # we prefer a minimal target as transitive dependency.
+            hash[name] = values.sort_by { |pt| pt.specs.count }
+          end
+          pod_targets.each do |target|
+            dependencies = transitive_dependencies_for_specs(target.specs, target.platform, all_specs).group_by(&:root)
+            target.dependent_targets = dependencies.map do |root_spec, deps|
+              pod_targets_by_name[root_spec.name].find do |t|
+                next false if t.platform.symbolic_name != target.platform.symbolic_name ||
+                    t.requires_frameworks? != target.requires_frameworks?
+                spec_names = t.specs.map(&:name)
+                deps.all? { |dep| spec_names.include?(dep.name) }
+              end
+            end
+          end
         else
           dedupe_cache = {}
-          pod_targets = specs_by_target.flat_map do |target_definition, specs|
-            grouped_specs = specs.group_by.group_by(&:root).values.uniq
-            grouped_specs.flat_map do |pod_specs|
+          specs_by_target.flat_map do |target_definition, specs|
+            grouped_specs = specs.group_by(&:root).values.uniq
+            pod_targets = grouped_specs.flat_map do |pod_specs|
               generate_pod_target([target_definition], pod_specs).scoped(dedupe_cache)
             end
+
+            pod_targets.each do |target|
+              dependencies = transitive_dependencies_for_specs(target.specs, target.platform, specs).group_by(&:root)
+              target.dependent_targets = pod_targets.reject { |t| dependencies[t.root_spec].nil? }
+            end
           end
         end
-        pod_targets.each do |target|
-          target.dependent_targets = transitive_dependencies_for_pod_target(target, pod_targets)
-        end
       end
 
-      # Finds the names of the Pods upon which the given target _transitively_
-      # depends.
+      # Returns the specs upon which the given specs _transitively_ depend.
       #
       # @note: This is implemented in the analyzer, because we don't have to
       #        care about the requirements after dependency resolution.
       #
-      # @param  [PodTarget] pod_target
-      #         The pod target, whose dependencies should be returned.
+      # @param  [Array<Specification>] specs
+      #         The specs, whose dependencies should be returned.
       #
-      # @param  [Array<PodTarget>] targets
-      #         All pod targets, which are integrated alongside.
+      # @param  [Platform] platform
+      #         The platform for which the dependencies should be returned.
       #
-      # @return [Array<PodTarget>]
+      # @param  [Array<Specification>] all_specs
+      #         All specifications which are installed alongside.
       #
-      def transitive_dependencies_for_pod_target(pod_target, targets)
-        if targets.any?
-          dependent_targets = pod_target.dependencies.flat_map do |dep|
-            next [] if pod_target.pod_name == dep
-            targets.select { |t| t.pod_name == dep }
-          end
-          remaining_targets = targets - dependent_targets
-          dependent_targets += dependent_targets.flat_map do |target|
-            transitive_dependencies_for_pod_target(target, remaining_targets)
-          end
-          dependent_targets.uniq
-        else
-          []
-        end
+      # @return [Array<Specification>]
+      #
+      def transitive_dependencies_for_specs(specs, platform, all_specs)
+        return [] if specs.empty? || all_specs.empty?
+        dependent_specs = specs.flat_map do |spec|
+          spec.consumer(platform).dependencies.flat_map do |dependency|
+            all_specs.find do |s|
+              next false if specs.include?(s)
+              s.name == dependency.name
+            end
+          end.compact
+        end.uniq
+        remaining_specs = all_specs - dependent_specs
+        dependent_specs + transitive_dependencies_for_specs(dependent_specs, platform, remaining_specs)
       end
 
       # Create a target for each spec group

spec/unit/installer/analyzer_spec.rb

@@ -4,6 +4,11 @@ module Pod
   describe Installer::Analyzer do
     describe 'Analysis' do
       before do
+        repos = [fixture('spec-repos/test_repo'), 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)
+
         @podfile = Pod::Podfile.new do
           platform :ios, '6.0'
           project 'SampleProject/SampleProject'
@@ -162,37 +167,63 @@ module Pod
         target.platform.to_s.should == 'iOS 6.0'
       end
 
-      it 'generates the set of dependent pod targets' do
-        @podfile = Pod::Podfile.new do
-          platform :ios, '8.0'
-          project 'SampleProject/SampleProject'
-          pod 'RestKit', '~> 0.23.0'
-          target 'TestRunner' do
-            pod 'RestKit/Testing', '~> 0.23.0'
+      describe 'dependent pod targets' do
+        it 'picks transitive dependencies up' do
+          @podfile = Pod::Podfile.new do
+            platform :ios, '8.0'
+            project 'SampleProject/SampleProject'
+            pod 'RestKit', '~> 0.23.0'
+            target 'TestRunner' do
+              pod 'RestKit/Testing', '~> 0.23.0'
+            end
           end
+          @analyzer = Pod::Installer::Analyzer.new(config.sandbox, @podfile, nil)
+          result = @analyzer.analyze
+          result.targets.count.should == 1
+          target = result.targets.first
+          restkit_target = target.pod_targets.find { |pt| pt.pod_name == 'RestKit' }
+          restkit_target.dependent_targets.map(&:pod_name).sort.should == %w(
+            AFNetworking
+            ISO8601DateFormatterValueTransformer
+            RKValueTransformers
+            SOCKit
+            TransitionKit
+          )
+          restkit_target.dependent_targets.all?(&:scoped).should.be.true
         end
-        @analyzer = Pod::Installer::Analyzer.new(config.sandbox, @podfile, nil)
-        @analyzer.analyze.targets.count.should == 1
-        target = @analyzer.analyze.targets.first
-        restkit_target = target.pod_targets.find { |pt| pt.pod_name == 'RestKit' }
-        restkit_target.dependent_targets.map(&:pod_name).sort.should == %w(
-          AFNetworking
-          ISO8601DateFormatterValueTransformer
-          RKValueTransformers
-          SOCKit
-          TransitionKit
-        )
-        restkit_target.dependent_targets.all?(&:scoped).should.be.true
-      end
 
-      describe 'deduplication' do
-        before do
-          repos = [fixture('spec-repos/test_repo'), 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)
+        it 'picks the right variants up when there are multiple' do
+          @podfile = Pod::Podfile.new do
+            source SpecHelper.test_repo_url
+            platform :ios, '8.0'
+            project 'SampleProject/SampleProject'
+
+            # The order of target definitions is important for this test.
+            target 'TestRunner' do
+              pod 'OrangeFramework'
+              pod 'matryoshka/Foo'
+            end
+
+            target 'SampleProject' do
+              pod 'OrangeFramework'
+            end
+          end
+          @analyzer = Pod::Installer::Analyzer.new(config.sandbox, @podfile, nil)
+          result = @analyzer.analyze
+
+          result.targets.count.should == 2
+
+          pod_target = result.targets[0].pod_targets.find { |pt| pt.pod_name == 'OrangeFramework' }
+          pod_target.dependent_targets.count == 1
+          pod_target.dependent_targets.first.specs.map(&:name).should == %w(
+            matryoshka
+            matryoshka/Outer
+            matryoshka/Outer/Inner
+          )
         end
+      end
 
+      describe 'deduplication' do
         it 'deduplicate targets if possible' do
           podfile = Pod::Podfile.new do
             source SpecHelper.test_repo_url