Refactoring -- Decoupling gem source logic from finder

11 files changed, 120 insertions, 81 deletions

diff --git a/lib/bundler/finder.rb b/lib/bundler/finder.rb
index 9c6e79431d..b77ca65709 100644
--- a/lib/bundler/finder.rb
+++ b/lib/bundler/finder.rb
@@ -18,23 +18,6 @@ module Bundler
       @sources = sources
     end
 
-    # Figures out the best possible configuration of gems that satisfies
-    # the list of passed dependencies and any child dependencies without
-    # causing any gem activation errors.
-    #
-    # ==== Parameters
-    # *dependencies<Gem::Dependency>:: The list of dependencies to resolve
-    #
-    # ==== Returns
-    # <GemBundle>,nil:: If the list of dependencies can be resolved, a
-    #   collection of gemspecs is returned. Otherwise, nil is returned.
-    def resolve(*dependencies)
-      Bundler.logger.info "Calculating dependencies..."
-
-      resolved = Resolver.resolve(dependencies, self)
-      resolved && GemBundle.new(resolved)
-    end
-
     # Searches for a gem that matches the dependency
     #
     # ==== Parameters
diff --git a/lib/bundler/manifest.rb b/lib/bundler/manifest.rb
index ca05d46223..dbd16b5940 100644
--- a/lib/bundler/manifest.rb
+++ b/lib/bundler/manifest.rb
@@ -6,25 +6,25 @@ module Bundler
   class Manifest
     attr_reader :sources, :dependencies, :path
 
-    def initialize(sources, dependencies, bindir, repository_path, rubygems, system_gems)
+    def initialize(sources, dependencies, bindir, path, rubygems, system_gems)
       @sources      = sources
       @dependencies = dependencies
       @bindir       = bindir
-      @repository   = Repository.new(repository_path)
+      @path         = path
       @rubygems     = rubygems
       @system_gems  = system_gems
     end
 
     def install(update)
       fetch(update)
-      @repository.install_cached_gems(:bin_dir => @bindir || @repository.path.join("bin"))
-      cleanup_removed_gems
-      create_environment_files(@repository.path.join("environments"))
+      repository.install_cached_gems(:bin_dir => @bindir || repository.path.join("bin"))
+      repository.cleanup(gems)
+      create_environment_files(repository.path.join("environments"))
       Bundler.logger.info "Done."
     end
 
     def activate(environment = "default")
-      require @repository.path.join("environments", "#{environment}.rb")
+      require repository.path.join("environments", "#{environment}.rb")
     end
 
     def require_all
@@ -37,7 +37,7 @@ module Bundler
       deps     = dependencies
       deps     = deps.select { |d| d.in?(environment) } if environment
       deps     = deps.map { |d| d.to_gem_dependency }
-      Resolver.resolve(deps, @repository.source_index)
+      Resolver.resolve(deps, repository.source_index)
     end
     alias gems gems_for
 
@@ -48,16 +48,23 @@ module Bundler
 
   private
 
+    def finder
+      @finder ||= Finder.new(*sources)
+    end
+
+    def repository
+      @repository ||= Repository.new(@path, @bindir)
+    end
+
     def fetch(update)
       return unless update || !all_gems_installed?
 
-      finder = Finder.new(*sources)
-      unless bundle = finder.resolve(*gem_dependencies)
+      unless bundle = Resolver.resolve(gem_dependencies, finder)
         gems = @dependencies.map {|d| "  #{d.to_s}" }.join("\n")
         raise VersionConflict, "No compatible versions could be found for:\n#{gems}"
       end
 
-      bundle.download(@repository)
+      bundle.download(repository)
     end
 
     def gem_dependencies
@@ -67,7 +74,7 @@ module Bundler
     def all_gems_installed?
       downloaded_gems = {}
 
-      Dir[@repository.path.join("cache", "*.gem")].each do |file|
+      Dir[repository.path.join("cache", "*.gem")].each do |file|
         file =~ /\/([^\/]+)-([\d\.]+)\.gem$/
         name, version = $1, $2
         downloaded_gems[name] = Gem::Version.new(version)
@@ -79,22 +86,6 @@ module Bundler
       end
     end
 
-    def cleanup_removed_gems
-      glob = gems.map { |g| g.full_name }.join(',')
-      base = @repository.path.join("{cache,specifications,gems}")
-
-      (Dir[base.join("*")] - Dir[base.join("{#{glob}}{.gemspec,.gem,}")]).each do |file|
-        Bundler.logger.info "Deleting gem: #{File.basename(file, ".gem")}" if File.basename(file) =~ /\.gem$/
-        FileUtils.rm_rf(file)
-      end
-
-      glob = gems.map { |g| g.executables }.flatten.join(',')
-      (Dir[@bindir.join("*")] - Dir[@bindir.join("{#{glob}}")]).each do |file|
-        Bundler.logger.info "Deleting bin file: #{File.basename(file)}"
-        FileUtils.rm_rf(file)
-      end
-    end
-
     def create_environment_files(path)
       FileUtils.mkdir_p(path)
       environments.each do |environment|
diff --git a/lib/bundler/manifest_file.rb b/lib/bundler/manifest_file.rb
index 2cad47f63e..7502c912e0 100644
--- a/lib/bundler/manifest_file.rb
+++ b/lib/bundler/manifest_file.rb
@@ -2,7 +2,7 @@ module Bundler
   class DefaultManifestNotFound < StandardError; end
 
   class ManifestFile
-    attr_reader :sources, :dependencies
+    attr_reader :dependencies
     attr_accessor :gem_path, :bindir, :rubygems, :system_gems
 
     def self.load(filename = nil)
@@ -10,11 +10,12 @@ module Bundler
     end
 
     def initialize(filename)
-      @filename      = filename
-      @sources       = [Source.new("http://gems.rubyforge.org")]
-      @dependencies  = []
-      @rubygems      = true
-      @system_gems   = true
+      @filename        = filename
+      @default_sources = [Source.new("http://gems.rubyforge.org")]
+      @sources         = []
+      @dependencies    = []
+      @rubygems        = true
+      @system_gems     = true
     end
 
     def load
@@ -30,6 +31,19 @@ module Bundler
       manifest.install(update)
     end
 
+    def sources
+      @sources + @default_sources
+    end
+
+    def add_source(source)
+      @sources << source
+    end
+
+    def clear_sources
+      @sources.clear
+      @default_sources.clear
+    end
+
     def setup_environment
       unless @system_gems
         ENV["GEM_HOME"] = @gem_path
diff --git a/lib/bundler/repository.rb b/lib/bundler/repository.rb
index 1ca53f43ab..5cdc786a1c 100644
--- a/lib/bundler/repository.rb
+++ b/lib/bundler/repository.rb
@@ -5,8 +5,9 @@ module Bundler
 
     attr_reader :path
 
-    def initialize(path)
-      @path = Pathname.new(path)
+    def initialize(path, bindir = nil)
+      @path   = Pathname.new(path)
+      @bindir = Pathname.new(bindir) || @path.join("bin")
       unless valid?
         raise InvalidRepository, "'#{path}' is not a valid gem repository"
       end
@@ -54,6 +55,25 @@ module Bundler
       end
     end
 
+    def cleanup(gems)
+      glob = gems.map { |g| g.full_name }.join(',')
+      base = path.join("{cache,specifications,gems}")
+
+      (Dir[base.join("*")] - Dir[base.join("{#{glob}}{.gemspec,.gem,}")]).each do |file|
+        if File.basename(file) =~ /\.gem$/
+          name = File.basename(file, '.gem')
+          Bundler.logger.info "Deleting gem: #{name}"
+        end
+        FileUtils.rm_rf(file)
+      end
+
+      glob = gems.map { |g| g.executables }.flatten.join(',')
+      (Dir[@bindir.join("*")] - Dir[@bindir.join("{#{glob}}")]).each do |file|
+        Bundler.logger.info "Deleting bin file: #{File.basename(file)}"
+        FileUtils.rm_rf(file)
+      end
+    end
+
   private
 
     def cache_path
diff --git a/lib/bundler/resolver.rb b/lib/bundler/resolver.rb
index 0966a877f6..309e4ac00d 100644
--- a/lib/bundler/resolver.rb
+++ b/lib/bundler/resolver.rb
@@ -24,13 +24,25 @@ module Bundler
 
     attr_reader :errors
 
+    # Figures out the best possible configuration of gems that satisfies
+    # the list of passed dependencies and any child dependencies without
+    # causing any gem activation errors.
+    #
+    # ==== Parameters
+    # *dependencies<Gem::Dependency>:: The list of dependencies to resolve
+    #
+    # ==== Returns
+    # <GemBundle>,nil:: If the list of dependencies can be resolved, a
+    #   collection of gemspecs is returned. Otherwise, nil is returned.
     def self.resolve(requirements, index = Gem.source_index)
+      Bundler.logger.info "Calculating dependencies..."
+
       resolver = new(index)
       result = catch(:success) do
         resolver.resolve(requirements, {})
         nil
       end
-      result && result.values
+      result && GemBundle.new(result.values)
     end
 
     def initialize(index)
diff --git a/lib/bundler/runtime.rb b/lib/bundler/runtime.rb
index 35c600d35a..3cc87826f5 100644
--- a/lib/bundler/runtime.rb
+++ b/lib/bundler/runtime.rb
@@ -17,7 +17,7 @@ module Bundler
     end
 
     def initialize(manifest_file)
-      @manifest_file = manifest_file
+      @manifest_file  = manifest_file
     end
 
     def bundle_path(path)
@@ -43,12 +43,12 @@ module Bundler
     def source(source)
       source = Source.new(source)
       unless @manifest_file.sources.include?(source)
-        @manifest_file.sources << source
+        @manifest_file.add_source(source)
       end
     end
 
-    def sources
-      @manifest_file.sources
+    def clear_sources
+      @manifest_file.clear_sources
     end
 
     def gem(name, *args)
diff --git a/spec/bundler/cli_spec.rb b/spec/bundler/cli_spec.rb
index 25dc6646d2..5e48e3e1f6 100644
--- a/spec/bundler/cli_spec.rb
+++ b/spec/bundler/cli_spec.rb
@@ -4,7 +4,7 @@ describe "Bundler::CLI" do
   describe "it working" do
     before(:each) do
       build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         gem "rake"
         gem "extlib"
@@ -89,7 +89,7 @@ describe "Bundler::CLI" do
     it "works when the manifest is in the root directory" do
       build_manifest_file tmp_file('manifest.rb'), <<-Gemfile
         bundle_path "gems"
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         gem "rake"
       Gemfile
@@ -102,7 +102,7 @@ describe "Bundler::CLI" do
     it "works when the manifest is in a different directory" do
       build_manifest_file tmp_file('config', 'manifest.rb'), <<-Gemfile
         bundle_path "../gems"
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         gem "rake"
       Gemfile
@@ -116,7 +116,7 @@ describe "Bundler::CLI" do
   describe "it working without rubygems" do
     before(:each) do
       build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         gem "rake"
         gem "extlib"
@@ -154,7 +154,7 @@ describe "Bundler::CLI" do
   describe "forcing an update" do
     it "forces checking for remote updates if --update is used" do
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "rack", "0.9.1"
@@ -162,7 +162,7 @@ describe "Bundler::CLI" do
       m.install
 
       build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "rack"
diff --git a/spec/bundler/fetcher_spec.rb b/spec/bundler/fetcher_spec.rb
index 67c9571821..969a36eb98 100644
--- a/spec/bundler/fetcher_spec.rb
+++ b/spec/bundler/fetcher_spec.rb
@@ -17,7 +17,7 @@ describe "Fetcher" do
 
   it "raises if the source does not exist" do
     m = build_manifest <<-Gemfile
-      sources.clear
+      clear_sources
       source "file://not/a/gem/source"
       gem "foo"
     Gemfile
@@ -26,7 +26,7 @@ describe "Fetcher" do
 
   it "raises if the source is not available" do
     m = build_manifest <<-Gemfile
-      sources.clear
+      clear_sources
       source "http://localhost"
       gem "foo"
     Gemfile
@@ -35,7 +35,7 @@ describe "Fetcher" do
 
   it "raises if the source is not a gem repository" do
     m = build_manifest <<-Gemfile
-      sources.clear
+      clear_sources
       source "http://google.com/not/a/gem/location"
       gem "foo"
     Gemfile
@@ -64,7 +64,7 @@ describe "Fetcher" do
 
   it "outputs a logger message when updating an index from source" do
     m = build_manifest <<-Gemfile
-      sources.clear
+      clear_sources
       source "file://#{gem_repo1}"
       source "file://#{gem_repo2}"
       gem "very-simple"
diff --git a/spec/bundler/installer_spec.rb b/spec/bundler/installer_spec.rb
index 09a19fce3b..998c9fae07 100644
--- a/spec/bundler/installer_spec.rb
+++ b/spec/bundler/installer_spec.rb
@@ -12,7 +12,7 @@ describe "Installing gems" do
 
     def setup
       @manifest = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "rails"
@@ -43,13 +43,14 @@ describe "Installing gems" do
       tmp_file("vendor", "gems", "fail").touch_p
       lambda {
         setup
+        @manifest.install
       }.should raise_error(Bundler::InvalidRepository)
     end
 
     it "installs the bins in the directory you specify" do
       tmp_file("omgbinz").mkdir
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         bin_path "#{tmp_file("omgbinz")}"
@@ -108,7 +109,7 @@ describe "Installing gems" do
 
     it "works with prerelease gems" do
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         gem "webrat", "0.4.4.racktest"
       Gemfile
@@ -133,7 +134,7 @@ describe "Installing gems" do
 
     it "compiles binary gems" do
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo2}"
         gem "json"
       Gemfile
diff --git a/spec/bundler/manifest_file_spec.rb b/spec/bundler/manifest_file_spec.rb
index 852bf63707..2f3baaaead 100644
--- a/spec/bundler/manifest_file_spec.rb
+++ b/spec/bundler/manifest_file_spec.rb
@@ -79,4 +79,22 @@ describe "Bundler::Manifest" do
     Dir.chdir(tmp_file("baz"))
     Bundler::ManifestFile.load.sources.should have(2).items
   end
+
+  it "inserts new sources before rubyforge" do
+    m = build_manifest <<-Gemfile
+      source "http://gems.github.com"
+    Gemfile
+    m.sources.map{|s| s.uri.to_s}.should ==
+      %w(http://gems.github.com http://gems.rubyforge.org)
+  end
+
+  it "inserts new sources at the end if the default has been removed" do
+    m = build_manifest <<-Gemfile
+      clear_sources
+      source "http://gems.rubyforge.org"
+      source "http://gems.github.com"
+    Gemfile
+    m.sources.map{|s| s.uri.to_s}.should ==
+      %w(http://gems.rubyforge.org http://gems.github.com)
+  end
 end
diff --git a/spec/bundler/manifest_spec.rb b/spec/bundler/manifest_spec.rb
index 68fbfa9342..c34343e15c 100644
--- a/spec/bundler/manifest_spec.rb
+++ b/spec/bundler/manifest_spec.rb
@@ -17,7 +17,7 @@ describe "Bundler::Manifest" do
 
     before(:each) do
       @manifest = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "rails", "2.3.2"
@@ -60,7 +60,7 @@ describe "Bundler::Manifest" do
       @manifest.install
 
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "rails", "2.3.2"
@@ -85,7 +85,7 @@ describe "Bundler::Manifest" do
       tmp_bindir("rackup").should exist
 
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "rails", "2.3.2"
@@ -117,7 +117,7 @@ describe "Bundler::Manifest" do
 
     it "raises a friendly exception if the manifest doesn't resolve" do
       build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "rails", "2.3.2"
@@ -138,7 +138,7 @@ describe "Bundler::Manifest" do
 
     it "makes gems available via Manifest#activate" do
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "very-simple", "1.0.0"
@@ -154,7 +154,7 @@ describe "Bundler::Manifest" do
 
     it "makes gems available" do
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "very-simple", "1.0.0"
@@ -172,7 +172,7 @@ describe "Bundler::Manifest" do
 
     it "is able to work system gems" do
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         gem "very-simple"
       Gemfile
@@ -184,7 +184,7 @@ describe "Bundler::Manifest" do
 
     it "it does not work with system gems if system gems have been disabled" do
       m = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         gem "very-simple"
         disable_system_gems
@@ -199,7 +199,7 @@ describe "Bundler::Manifest" do
       describe desc do
         before(:each) do
           m = build_manifest <<-Gemfile
-            sources.clear
+            clear_sources
             source "file://#{gem_repo1}"
             gem "very-simple"
             #{'disable_system_gems' if i == 1}
@@ -228,7 +228,7 @@ describe "Bundler::Manifest" do
   describe "environments" do
     before(:each) do
       @manifest = build_manifest <<-Gemfile
-        sources.clear
+        clear_sources
         source "file://#{gem_repo1}"
         source "file://#{gem_repo2}"
         gem "very-simple", "1.0.0", :only => "testing"