Merge duplicated dependencies in spec

lib/librarian/action/resolve.rb

@@ -23,14 +23,14 @@ def run
           manifests = changes.analyze
         end
 
-        dupes = spec.dependencies.group_by{ |e| e.name }.select { |k, v| v.size > 1 }
-        dupes = Hash[dupes] if dupes.is_a? Array # Ruby 1.8 support
-        unless dupes.empty?
-          raise Error, "Duplicated dependencies: #{dupes.values.flatten.map {|d| {d.name => d.source.to_s} }}"
+        spec.dependencies, duplicated = Dependency.remove_duplicate_dependencies(spec.dependencies)
+        duplicated.each do |name, dependencies_same_name|
+          environment.logger.info { "Dependency '#{name}' duplicated for module #{name}, merging: #{dependencies_same_name.map{|d| d.to_s}}" }
         end
 
         resolution = resolver.resolve(spec, manifests)
         persist_resolution(resolution)
+        resolution
       end
 
     private

lib/librarian/dependency.rb

@@ -185,6 +185,32 @@ def inconsistent_with?(other)
       !consistent_with?(other)
     end
 
+    class << self
+      # merge dependencies with the same name into one
+      # with the source of the first one and merged requirements
+      def merge_dependencies(dependencies)
+        requirement = Dependency::Requirement.new(*dependencies.map{|d| d.requirement})
+        dependencies.last.class.new(dependencies.last.name, requirement, dependencies.last.source)
+      end
+
+      # Avoid duplicated dependencies with different sources or requirements
+      # Return [merged dependnecies, duplicates as a map by name]
+      def remove_duplicate_dependencies(dependencies)
+        uniq = []
+        duplicated = {}
+        dependencies_by_name = dependencies.group_by{|d| d.name}
+        dependencies_by_name.map do |name, dependencies_same_name|
+          if dependencies_same_name.size > 1
+            duplicated[name] = dependencies_same_name
+            uniq << merge_dependencies(dependencies_same_name)
+          else
+            uniq << dependencies_same_name.first
+          end
+        end
+          [uniq, duplicated]
+      end
+    end
+
   private
 
     def assert_name_valid!(name)

lib/librarian/environment.rb

@@ -71,7 +71,7 @@ def specfile_path
     end
 
     def specfile
-      Specfile.new(self, specfile_path)
+      @specfile ||= Specfile.new(self, specfile_path)
     end
 
     def adapter_module

lib/librarian/manifest.rb

@@ -94,31 +94,10 @@ def fetch_version!
     end
 
     def fetch_dependencies!
-      remove_duplicate_dependencies(name, source.fetch_dependencies(name, version, extra))
-    end
-
-    # merge dependencies with the same name into one
-    # with the source of the first one and merged requirements
-    def merge_dependencies(dependencies)
-      requirement = Dependency::Requirement.new(*dependencies.map{|d| d.requirement})
-      dependencies.first.class.new(dependencies.first.name, requirement, dependencies.first.source)
-    end
-
-    # Avoid duplicated dependencies with different sources or requirements
-    def remove_duplicate_dependencies(module_name, dependencies)
-      uniq = []
-      dependencies_by_name = dependencies.group_by{|d| d.name}
-      dependencies_by_name.map do |name, dependencies_same_name|
-        if dependencies_same_name.size > 1
-          environment.logger.warn { "Dependency '#{name}' duplicated for module #{module_name}, trying to merge: #{dependencies_same_name.map{|d| d.to_s}}" }
-          merged = merge_dependencies(dependencies_same_name)
-          environment.logger.warn { "Dependency '#{name}' merged as #{merged}" }
-          uniq << merged
-        else
-          uniq << dependencies_same_name.first
-        end
+      dependencies, duplicated = Dependency.remove_duplicate_dependencies(source.fetch_dependencies(name, version, extra))
+      duplicated.each do |name, dependencies_same_name|
+        environment.logger.info { "Dependency '#{name}' duplicated for module #{module_name}, merging: #{dependencies_same_name.map{|d| d.to_s}}" }
       end
-      uniq
     end
 
     def _normalize_version(version)

lib/librarian/mock/environment.rb

@@ -1,13 +1,15 @@
 require "librarian/environment"
+require "librarian/ui"
 require "librarian/mock/dsl"
 require "librarian/mock/version"
+require 'thor'
 
 module Librarian
   module Mock
     class Environment < Environment
 
-      def install_path
-        nil
+      def ui
+        Librarian::UI::Shell.new(Thor::Shell::Basic.new)
       end
 
       def registry(options = nil, &block)

lib/librarian/spec.rb

@@ -2,7 +2,6 @@ module Librarian
   class Spec
 
     attr_accessor :sources, :dependencies, :exclusions
-    private :sources=, :dependencies=, :exclusions=
 
     def initialize(sources, dependencies, exclusions = [])
       self.sources = sources

lib/librarian/specfile.rb

@@ -12,7 +12,7 @@ def initialize(environment, path)
     end
 
     def read(precache_sources = [])
-      environment.dsl(path, precache_sources)
+      @spec ||= environment.dsl(path, precache_sources)
     end
 
   end

spec/unit/action/resolve_spec.rb

@@ -1,27 +1,44 @@
 require "librarian/error"
+require "librarian/logger"
 require "librarian/action/resolve"
+require "librarian/mock/environment"
 require "librarian/mock/source"
 
 module Librarian
   describe Action::Resolve do
 
     let(:options) { {} }
-    let(:spec) { double() }
-    let(:env) { double(:specfile => double(:read => spec)) }
+    let(:spec) { Spec.new([], dependencies, []) }
+    let(:env) { Librarian::Mock::Environment.new }
     let(:action) { described_class.new(env, options) }
+    let(:source1) { Librarian::Mock::Source::Mock.new(env, "source1", {}) }
+    let(:source2) { Librarian::Mock::Source::Mock.new(env, "source2", {}) }
+
+    before do
+      env.stub(:specfile => double(:read => spec))
+    end
 
     describe "#run" do
 
       describe "behavior" do
 
-        describe "fail with duplicated dependencies" do
+        describe "merge duplicated dependencies" do
           let(:options) { {:force => true} }
-          let(:dependency) { Dependency.new('dependency_name', '1.0.0', nil ) }
-          let(:dependencies) { [ dependency, dependency ] }
-          let(:spec) { double(:dependencies => dependencies) }
+          let(:dependency1) { Dependency.new('dependency_name', '1.0.0', source1) }
+          let(:dependency2) { Dependency.new('dependency_name', '1.0.0', source2) }
+          let(:dependencies) { [ dependency1, dependency2 ] }
+          let(:manifest) do
+            m = Manifest.new(source1, dependency1.name)
+            m.version = '1.0.0'
+            m.dependencies = []
+            m
+          end
 
-          it "should fail with duplicated dependencies" do
-            expect { action.run }.to raise_error(Error, /^Duplicated dependencies: /)
+          it "should merge duplicated dependencies" do
+            Dependency.any_instance.stub(:manifests => [manifest])
+            action.stub(:persist_resolution)
+            resolution = action.run
+            expect(resolution.dependencies).to eq([dependency2])
           end
 
         end