Merge pull request #155 from mbacovsky/8607_rubygems_fail

Fixes #8607 - remove unnecessary module dependency resolution

doc/installation.md

@@ -32,7 +32,7 @@ In each of these directories hammer is trying to load ```cli_config.yml``` and a
 the ```cli.modules.d``` subdirectory which is place for specific configuration of hammer modules a.k.a. plugins.
 
 Later directories and files have precedence if they redefine the same option. Files from ```cli.modules.d```
-are loaded in alphabetical order. The modules are loaded in alphabetical order which can be overriden with explicit dependences set in module ```gemspec```.
+are loaded in alphabetical order. The modules are loaded in alphabetical order which can be overriden with explicit requirement of dependences in the modules.
 
 ### Manual installation
 The packaged version of hammer copies the template to `/etc/hammer` for you.

lib/hammer_cli/exception_handler.rb

@@ -18,7 +18,6 @@ def mappings
         [RestClient::Unauthorized, :handle_unauthorized],
         [ApipieBindings::DocLoadingError, :handle_apipie_docloading_error],
         [ApipieBindings::MissingArgumentsError, :handle_apipie_missing_arguments_error],
-        [HammerCLI::ModuleCircularDependency, :handle_generic_config_error],
         [HammerCLI::ModuleDisabledButRequired, :handle_generic_config_error]
       ]
     end

lib/hammer_cli/exceptions.rb

@@ -3,7 +3,6 @@ module HammerCLI
   class CommandConflict < StandardError; end
   class OperationNotSupportedError < StandardError; end
   class ModuleLoadingError < StandardError; end
-  class ModuleCircularDependency < StandardError; end
   class ModuleDisabledButRequired < StandardError; end
 
 end

lib/hammer_cli/modules.rb

@@ -1,58 +1,9 @@
-require 'tsort'
 module HammerCLI
 
-  class ModulesList < Hash
-    include TSort
-
-    def tsort_each_node(&block)
-      each_key.sort.each(&block)
-    end
-
-    def tsort_each_child(node, &block)
-      fetch(node).each(&block)
-    end
-  end
-
   class Modules
 
     def self.names
-      # add dependencies
-      modules = find_dependencies({}, enabled_modules)
-      # sort with the deps in mind
-      ModulesList[modules].tsort
-    rescue TSort::Cyclic => e
-      raise HammerCLI::ModuleCircularDependency.new(_("Unable to load modules: Circular dependency detected (%s)") % e.message)
-    end
-
-
-    def self.find_dependencies(dependencies, module_list)
-      new_deps = []
-
-      # add inspected modules in current level (depth)
-      dependencies.merge(Hash[module_list.map{ |m| [m, []] }])
-
-      # lookup dependencies
-      module_list.each do |mod|
-        deps = dependencies_for(mod)
-        logger.debug(_("Module depenedency detected: %{mod} requires %{deps}") % { :mod => mod, :deps => deps.join(', ') }) unless deps.empty?
-        dependencies[mod] = deps # update deps
-        # check new/disabled deps
-        deps.each do |dep|
-          if !dependencies.has_key?(dep)
-            if HammerCLI::Settings.get(dep.gsub(/^hammer_cli_/, ''), :enable_module) == false
-              raise HammerCLI::ModuleDisabledButRequired.new(_("Module %{mod} depends on module %{dep} which is disabled in configuration") % { :mod => mod, :dep => dep })
-            end
-            new_deps << dep
-          end
-        end
-      end
-      dependencies = find_dependencies(dependencies, new_deps) unless new_deps.empty?
-      dependencies
-    end
-
-    def self.dependencies_for(module_name)
-      mod = Gem::Specification.find_by_name(module_name)
-      mod.dependencies.select{ |dep| dep.name =~ /^hammer_cli_.*/ }.map(&:name)
+      enabled_modules.sort
     end
 
     def self.enabled_modules
@@ -73,6 +24,22 @@ def self.enabled_modules
       end
     end
 
+    def self.disabled_modules
+      HammerCLI::Settings.dump.inject([]) do |names, (mod_name, mod_config)|
+        if is_module_config?(mod_config)
+          mod = "hammer_cli_#{mod_name}"
+          names << mod unless mod_config[:enable_module]
+        end
+        names
+      end
+    end
+
+    def self.loaded_modules
+      Object.constants. \
+        select{ |c| c.to_s =~ /\AHammerCLI[A-Z]./ && Object.const_get(c).class == Module }. \
+        map{ |m| m.to_s.underscore }
+    end
+
     def self.find_by_name(name)
       possible_names = [
         name.camelize,
@@ -115,6 +82,11 @@ def self.load_all
       HammerCLI::Modules.names.each do |m|
         Modules.load(m)
       end
+      loaded_for_deps = loaded_modules & disabled_modules
+      unless loaded_for_deps.empty?
+        message = _("Error: Some of the required modules are disabled in configuration: %s ") % loaded_for_deps.join(', ')
+        raise HammerCLI::ModuleDisabledButRequired.new(message)
+      end
     end
 
     protected

lib/hammer_cli/utils.rb

@@ -24,6 +24,16 @@ def indent_with(indent_str)
     gsub(/^/, indent_str)
   end
 
+  def underscore
+    word = self.dup
+    word.gsub!(/::/, '/')
+    word.gsub!(/([A-Z]+)([A-Z][a-z])/,'\1_\2')
+    word.gsub!(/([a-z\d])([A-Z])/,'\1_\2')
+    word.tr!("-", "_")
+    word.downcase!
+    word
+  end
+
   def constantize
     raise NameError, "Can't constantize empty string" if self.empty?
     HammerCLI.constant_path(self)[-1]

test/unit/modules_test.rb

@@ -28,7 +28,6 @@ def self.version
 
   describe "names" do
     it "must return list of modules" do
-      HammerCLI::Modules.stubs(:dependencies_for).returns([])
       HammerCLI::Modules.names.must_equal ["hammer_cli_jerry", "hammer_cli_tom"]
     end
 
@@ -43,52 +42,9 @@ def self.version
         :tom => {},
         :modules => ['hammer_cli_tom', 'hammer_cli_jerry'],
       })
-      HammerCLI::Modules.stubs(:dependencies_for).returns([])
       HammerCLI::Modules.names.must_equal ["hammer_cli_jerry", "hammer_cli_tom"]
     end
 
-    it "must resolve module depndences" do
-      HammerCLI::Modules.stubs(:dependencies_for).returns([])
-      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom'])
-      HammerCLI::Modules.names.must_equal ["hammer_cli_tom", "hammer_cli_jerry"]
-    end
-
-    it "must detect circular dependences" do
-      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom'])
-      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_tom').returns(['hammer_cli_jerry'])
-      proc { HammerCLI::Modules.names }.must_raise HammerCLI::ModuleCircularDependency
-    end
-
-    it "must sort modules with dependency depth > 1" do
-      HammerCLI::Settings.clear
-      HammerCLI::Settings.load({
-        :tom => { :enable_module => true },
-        :jerry => { :enable_module => true },
-        :cherie => { :enable_module => true }
-      })
-      HammerCLI::Modules.stubs(:dependencies_for).returns([])
-      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom'])
-      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_cherie').returns(['hammer_cli_jerry'])
-      HammerCLI::Modules.names.must_equal ["hammer_cli_tom", "hammer_cli_jerry", "hammer_cli_cherie"]
-    end
-
-    it "must handle dependency on module not mentioned in configuration" do
-      HammerCLI::Modules.stubs(:dependencies_for).returns([])
-      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom', 'hammer_cli_cherie'])
-      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_cherie').returns(['hammer_cli_quacker'])
-      HammerCLI::Modules.names.must_equal ["hammer_cli_quacker", "hammer_cli_cherie", "hammer_cli_tom", "hammer_cli_jerry"]
-    end
-
-    it "must detect dependency on disabled module" do
-      HammerCLI::Settings.clear
-      HammerCLI::Settings.load({
-        :tom => { :enable_module => false },
-        :jerry => { :enable_module => true },
-      })
-      HammerCLI::Modules.stubs(:dependencies_for).returns([])
-      HammerCLI::Modules.stubs(:dependencies_for).with('hammer_cli_jerry').returns(['hammer_cli_tom'])
-      proc { HammerCLI::Modules.names }.must_raise HammerCLI::ModuleDisabledButRequired
-    end
   end
 
   describe "find by name" do
@@ -108,12 +64,21 @@ def self.version
   describe "load all modules" do
 
     it "must call load for each module" do
-      HammerCLI::Modules.stubs(:dependencies_for).returns([])
       HammerCLI::Modules.expects(:load).with("hammer_cli_tom")
       HammerCLI::Modules.expects(:load).with("hammer_cli_jerry")
       HammerCLI::Modules.load_all
     end
 
+    it "must detect dependency on disabled module" do
+      HammerCLI::Settings.clear
+      HammerCLI::Settings.load({
+        :tom => { :enable_module => false },
+        :jerry => { :enable_module => true },
+      })
+      HammerCLI::Modules.stubs(:load).returns(nil)
+      HammerCLI::Modules.stubs(:loaded_modules).returns(['hammer_cli_jerry', 'hammer_cli_tom'])
+      proc { HammerCLI::Modules.load_all }.must_raise HammerCLI::ModuleDisabledButRequired
+    end
   end
 
   describe "load a module" do

test/unit/utils_test.rb

@@ -51,6 +51,23 @@ class X
 
   end
 
+
+  describe "underscore" do
+
+    it "converts camelized string to underscore" do
+      "OneTwoThree".underscore.must_equal "one_two_three"
+    end
+
+    it "converts full class path name to underscore with slashes" do
+      "HammerCLI::SomeClass".underscore.must_equal "hammer_cli/some_class"
+    end
+
+    it "converts dashes to underscores" do
+      "Re-Read".underscore.must_equal "re_read"
+    end
+
+  end
+
   describe "indent" do
 
     it "indents single line string" do