Merge pull request #422 from CocoaPods/seg-dependency-merge-podspec-repo

[Dependency] Have #merge take into account podspec repos

CHANGELOG.md

@@ -10,7 +10,9 @@
 
 ##### Bug Fixes
 
-* None.  
+* The `Dependency#merge` method takes into account any `podspec_repo`s the dependencies
+  may have set.  
+  [Samuel Giddins](https://github.com/segiddins)
 
 
 ## 1.4.0 (2018-01-18)

lib/cocoapods-core/dependency.rb

@@ -95,8 +95,7 @@ def initialize(name = nil, *requirements)
             raise Informative, 'A dependency with a specified podspec repo may ' \
               "not include other source parameters (#{name})."
           end
-        else
-          @external_source = additional_params
+        elsif @external_source = additional_params
           unless requirements.empty?
             raise Informative, 'A dependency with an external source may not ' \
               "specify version requirements (#{name})."
@@ -216,7 +215,8 @@ def ==(other)
       self.class == other.class &&
         name == other.name &&
         requirement == other.requirement &&
-        external_source == other.external_source
+        external_source == other.external_source &&
+        podspec_repo == other.podspec_repo
     end
     alias_method :eql?, :==
 
@@ -249,24 +249,41 @@ def merge(other)
       unless name == other.name
         raise ArgumentError, "#{self} and #{other} have different names"
       end
+
       default   = Requirement.default
       self_req  = requirement
       other_req = other.requirement
 
-      dep = if other_req == default
-              self.class.new(name, self_req)
+      req = if other_req == default
+              self_req
             elsif self_req == default
-              self.class.new(name, other_req)
+              other_req
             else
-              self.class.new(name, self_req.as_list.concat(other_req.as_list))
+              self_req.as_list.concat(other_req.as_list)
             end
 
+      opts = {}
+
       if external_source || other.external_source
-        self_external_source  = external_source || {}
-        other_external_source = other.external_source || {}
-        dep.external_source = self_external_source.merge(other_external_source)
+        opts.
+          merge!(external_source || {}).
+          merge!(other.external_source || {})
+
+        req_to_set = req
+        req = []
+      end
+
+      if podspec_repo && other.podspec_repo && podspec_repo != other.podspec_repo
+        raise ArgumentError, "#{self} and #{other} have different podspec repos"
+      end
+
+      if repo = podspec_repo || other.podspec_repo
+        opts[:source] = repo
+      end
+
+      self.class.new(name, *req, opts).tap do |dep|
+        dep.instance_variable_set(:@requirement, Requirement.create(req_to_set)) if req_to_set
       end
-      dep
     end
 
     # Whether the dependency has any pre-release requirements

spec/dependency_spec.rb

@@ -185,6 +185,14 @@ module Pod
         dep1.should == dep3
       end
 
+      it 'is equal to another dependency if `podspec_repo` is the same' do
+        dep1 = Dependency.new('bananas', :source => 'GIT-URL')
+        dep2 = Dependency.new('bananas')
+        dep1.should.not == dep2
+        dep3 = Dependency.new('bananas', :source => 'GIT-URL')
+        dep1.should == dep3
+      end
+
       it 'supports Array#uniq' do
         d_1 = Dependency.new('bananas')
         d_2 = Dependency.new('bananas')
@@ -211,9 +219,14 @@ module Pod
       it 'it preserves the external source while merging with another dependency' do
         dep1 = Dependency.new('bananas', '1.9')
         dep2 = Dependency.new('bananas', :podspec => 'bananas')
+
         result = dep1.merge(dep2)
         result.should.be.external
         result.requirement.as_list.should == ['= 1.9']
+
+        result = dep2.merge(dep1)
+        result.should.be.external
+        result.requirement.as_list.should == ['= 1.9']
       end
 
       it 'raises if there is an attempt to merge with a dependency with a different name' do
@@ -224,6 +237,50 @@ module Pod
         end
       end
 
+      it 'preserves the podspec repo while merging' do
+        dep1 = Dependency.new('bananas', '~> 1.9')
+        dep2 = Dependency.new('bananas', :source => 'https://source.git')
+
+        expected = Dependency.new('bananas', '~> 1.9', :source => 'https://source.git')
+        dep1.merge(dep2).should == expected
+        dep2.merge(dep1).should == expected
+      end
+
+      it 'raises if there is an attempt to merge with two different podspec repos' do
+        should.raise ArgumentError do
+          dep1 = Dependency.new('bananas', :source => 'https://other.git')
+          dep2 = Dependency.new('bananas', :source => 'https://source.git')
+
+          dep1.merge(dep2)
+        end
+      end
+
+      it 'allows merging if both dependencies have the same podspec repo' do
+        dep1 = Dependency.new('bananas', '~> 1.9', :source => 'https://source.git')
+        dep2 = Dependency.new('bananas', :source => 'https://source.git')
+
+        expected = Dependency.new('bananas', '~> 1.9', :source => 'https://source.git')
+        dep1.merge(dep2).should == expected
+        dep2.merge(dep1).should == expected
+      end
+
+      it 'raises when attempting to merge with a podspec repo and external source' do
+        should.raise Informative do
+          dep1 = Dependency.new('bananas', :git => 'https://other.git')
+          dep2 = Dependency.new('bananas', :source => 'https://source.git')
+
+          dep1.merge(dep2)
+        end
+      end
+
+      it 'prefers the right hand side when external source keys overlap' do
+        dep1 = Dependency.new('bananas', :git => 'https://other.git')
+        dep2 = Dependency.new('bananas', :git => 'https://other/1.git')
+
+        dep1.merge(dep2).should == Dependency.new('bananas', :git => 'https://other/1.git')
+        dep2.merge(dep1).should == Dependency.new('bananas', :git => 'https://other.git')
+      end
+
       #--------------------------------------#
 
       it 'matches a specification with the correct name' do