* lib/rubygems/commands/cleanup_command.rb: Clean all possible gems

using multiple passes.  Fixes RubyGems bug #422.  Refactored for
  maintainability.
* test/rubygems/test_gem_commands_cleanup_command.rb:  Test for above.


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@38698 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,10 @@
+Sat Jan  5 07:54:59 2013  Eric Hodel  <drbrain@segment7.net>
+
+	* lib/rubygems/commands/cleanup_command.rb:  Clean all possible gems
+	  using multiple passes.  Fixes RubyGems bug #422.  Refactored for
+	  maintainability.
+	* test/rubygems/test_gem_commands_cleanup_command.rb:  Test for above.
+
 Sat Jan  5 05:04:39 2013  KOSAKI Motohiro  <kosaki.motohiro@gmail.com>
 
 	* gc.c (vm_xrealloc): add a few comment why we avoid realloc(ptr,0).

lib/rubygems/commands/cleanup_command.rb

@@ -12,6 +12,14 @@ class Gem::Commands::CleanupCommand < Gem::Command
     add_option('-d', '--dryrun', "") do |value, options|
       options[:dryrun] = true
     end
+
+    @candidate_gems  = nil
+    @default_gems    = []
+    @full            = nil
+    @gems_to_cleanup = nil
+    @original_home   = nil
+    @original_path   = nil
+    @primary_gems    = nil
   end
 
   def arguments # :nodoc:
@@ -38,47 +46,99 @@ are not removed.
 
   def execute
     say "Cleaning up installed gems..."
-    primary_gems = {}
 
-    Gem::Specification.each do |spec|
+    if options[:args].empty? then
-      if primary_gems[spec.name].nil? or
+      done     = false
-         primary_gems[spec.name].version < spec.version then
+      last_set = nil
-        primary_gems[spec.name] = spec
+
+      until done do
+        clean_gems
+
+        this_set = @gems_to_cleanup.map { |spec| spec.full_name }.sort
+
+        done = this_set.empty? || last_set == this_set
+
+        last_set = this_set
+      end
+    else
+      clean_gems
+    end
+
+    say "Clean Up Complete"
+
+    if Gem.configuration.really_verbose then
+      skipped = @default_gems.map { |spec| spec.full_name }
+
+      say "Skipped default gems: #{skipped.join ', '}"
     end
   end
 
-    candidate_gems = unless options[:args].empty? then
+  def clean_gems
+    get_primary_gems
+    get_candidate_gems
+    get_gems_to_cleanup
+
+    @full = Gem::DependencyList.from_specs
+
+    deplist = Gem::DependencyList.new
+    @gems_to_cleanup.each do |spec| deplist.add spec end
+
+    deps = deplist.strongly_connected_components.flatten
+
+    @original_home = Gem.dir
+    @original_path = Gem.path
+
+    deps.reverse_each do |spec|
+      uninstall_dep spec
+    end
+
+    Gem::Specification.reset
+  end
+
+  def get_candidate_gems
+    @candidate_gems = unless options[:args].empty? then
                         options[:args].map do |gem_name|
                           Gem::Specification.find_all_by_name gem_name
                         end.flatten
                       else
                         Gem::Specification.to_a
                       end
+  end
 
-    gems_to_cleanup = candidate_gems.select { |spec|
+  def get_gems_to_cleanup
-      !spec.default_gem? and
+    gems_to_cleanup = @candidate_gems.select { |spec|
-        primary_gems[spec.name].version != spec.version
+      @primary_gems[spec.name].version != spec.version
     }
 
-    full = Gem::DependencyList.from_specs
+    default_gems, gems_to_cleanup = gems_to_cleanup.partition { |spec|
+      spec.default_gem?
+    }
 
-    deplist = Gem::DependencyList.new
+    @default_gems += default_gems
-    gems_to_cleanup.uniq.each do |spec| deplist.add spec end
+    @default_gems.uniq!
+    @gems_to_cleanup = gems_to_cleanup.uniq
+  end
 
-    deps = deplist.strongly_connected_components.flatten.reverse
+  def get_primary_gems
+    @primary_gems = {}
 
-    original_home = Gem.dir
+    Gem::Specification.each do |spec|
-    original_path = Gem.path
+      if @primary_gems[spec.name].nil? or
+         @primary_gems[spec.name].version < spec.version then
+        @primary_gems[spec.name] = spec
+      end
+    end
+  end
 
-    deps.each do |spec|
+  def uninstall_dep spec
-      next unless full.ok_to_remove?(spec.full_name)
+    return unless @full.ok_to_remove?(spec.full_name)
 
     if options[:dryrun] then
       say "Dry Run Mode: Would uninstall #{spec.full_name}"
-      else
+      return
-        say "Attempting to uninstall #{spec.full_name}"
+    end
 
-        options[:args] = [spec.name]
+    say "Attempting to uninstall #{spec.full_name}"
 
     uninstall_options = {
       :executables => false,
@@ -96,21 +156,9 @@ are not removed.
       say "Unable to uninstall #{spec.full_name}:"
       say "\t#{e.class}: #{e.message}"
     end
-      end
+  ensure
-
+    # Restore path Gem::Uninstaller may have changed
-      # Restore path Gem::Uninstaller may have change
+    Gem.use_paths @original_home, *@original_path
-      Gem.use_paths(original_home, *original_path)
-    end
-
-    say "Clean Up Complete"
-
-    if Gem.configuration.really_verbose then
-      skipped = candidate_gems.
-        select { |spec| spec.default_gem? }.
-        map { |spec| spec.full_name}
-
-      say "Skipped default gems: #{skipped.join ', '}"
-    end
   end
 
 end

test/rubygems/test_gem_commands_cleanup_command.rb

@@ -23,6 +23,21 @@ class TestGemCommandsCleanupCommand < Gem::TestCase
     refute_path_exists @a_1.gem_dir
   end
 
+  def test_execute_all_dependencies
+    @b_1 = quick_spec 'b', 1 do |s| s.add_dependency 'a', '1' end
+    @b_2 = quick_spec 'b', 2 do |s| s.add_dependency 'a', '2' end
+
+    install_gem @b_1
+    install_gem @b_2
+
+    @cmd.options[:args] = []
+
+    @cmd.execute
+
+    refute_path_exists @a_1.gem_dir
+    refute_path_exists @b_1.gem_dir
+  end
+
   def test_execute_all
     gemhome2 = File.join @tempdir, 'gemhome2'