Merge pull request #3194 from TimMoore/issue-3167-source-blocks-deployment

Fix bundle install --deployment with source blocks

6 files changed, 62 insertions, 23 deletions

diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 4d483774ff..8e7b4d9d9c 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -313,7 +313,7 @@ module Bundler
       deleted = []
       changed = []
 
-      gemfile_sources = sources.all_sources
+      gemfile_sources = sources.lock_sources
       if @locked_sources != gemfile_sources
         new_sources = gemfile_sources - @locked_sources
         deleted_sources = @locked_sources - gemfile_sources
@@ -451,12 +451,11 @@ module Bundler
 
       # Get the Rubygems sources from the Gemfile.lock
       locked_gem_sources = @locked_sources.select { |s| s.kind_of?(Source::Rubygems) }
-      # Get the Rubygems sources from the Gemfile
-      actual_gem_sources = @sources.rubygems_sources
+      # Get the Rubygems remotes from the Gemfile
+      actual_remotes = sources.rubygems_remotes
 
       # If there is a Rubygems source in both
-      unless locked_gem_sources.empty? && actual_gem_sources.empty?
-        actual_remotes = actual_gem_sources.map(&:remotes).flatten.uniq
+      if !locked_gem_sources.empty? && !actual_remotes.empty?
         locked_gem_sources.each do |locked_gem|
           # Merge the remotes from the Gemfile into the Gemfile.lock
           changes = changes | locked_gem.replace_remotes(actual_remotes)
@@ -466,11 +465,9 @@ module Bundler
       # Replace the sources from the Gemfile with the sources from the Gemfile.lock,
       # if they exist in the Gemfile.lock and are `==`. If you can't find an equivalent
       # source in the Gemfile.lock, use the one from the Gemfile.
-      sources.replace_sources!(@locked_sources)
-      gemfile_sources = sources.all_sources
-      changes = changes | (Set.new(gemfile_sources) != Set.new(@locked_sources))
+      changes = changes | sources.replace_sources!(@locked_sources)
 
-      gemfile_sources.each do |source|
+      sources.all_sources.each do |source|
         # If the source is unlockable and the current command allows an unlock of
         # the source (for example, you are doing a `bundle update <foo>` of a git-pinned
         # gem), unlock it. For git sources, this means to unlock the revision, which
diff --git a/lib/bundler/source/rubygems.rb b/lib/bundler/source/rubygems.rb
index f3b8b7c83b..044b363244 100644
--- a/lib/bundler/source/rubygems.rb
+++ b/lib/bundler/source/rubygems.rb
@@ -52,9 +52,9 @@ module Bundler
 
       def to_lock
         out = "GEM\n"
-        out << remotes.map { |remote|
-          "  remote: #{suppress_configured_credentials remote}\n"
-        }.join
+        remotes.reverse_each do |remote|
+          out << "  remote: #{suppress_configured_credentials remote}\n"
+        end
         out << "  specs:\n"
       end
 
diff --git a/lib/bundler/source_list.rb b/lib/bundler/source_list.rb
index 0d9fb516c8..05e2251fec 100644
--- a/lib/bundler/source_list.rb
+++ b/lib/bundler/source_list.rb
@@ -1,14 +1,13 @@
 module Bundler
   class SourceList
     attr_reader :path_sources,
-                :git_sources,
-                :rubygems_sources
+                :git_sources
 
     def initialize
       @path_sources       = []
       @git_sources        = []
       @rubygems_aggregate = Source::Rubygems.new
-      @rubygems_sources   = [@rubygems_aggregate]
+      @rubygems_sources   = []
     end
 
     def add_path_source(options = {})
@@ -28,6 +27,14 @@ module Bundler
       @rubygems_aggregate
     end
 
+    def rubygems_sources
+      @rubygems_sources + [@rubygems_aggregate]
+    end
+
+    def rubygems_remotes
+      rubygems_sources.map(&:remotes).flatten.uniq
+    end
+
     def all_sources
       path_sources + git_sources + rubygems_sources
     end
@@ -42,11 +49,21 @@ module Bundler
     end
 
     def replace_sources!(replacement_sources)
-      [path_sources, git_sources, rubygems_sources].each do |source_list|
+      return true if replacement_sources.empty?
+
+      [path_sources, git_sources].each do |source_list|
         source_list.map! do |source|
           replacement_sources.find { |s| s == source } || source
         end
       end
+
+      replacement_rubygems =
+        replacement_sources.detect { |s| s.is_a?(Source::Rubygems) }
+      @rubygems_aggregate = replacement_rubygems
+
+      # Return true if there were changes
+      all_sources.to_set != replacement_sources.to_set ||
+        rubygems_remotes.to_set != replacement_rubygems.remotes.to_set
     end
 
     def cached!
@@ -74,7 +91,7 @@ module Bundler
     end
 
     def combine_rubygems_sources
-      Source::Rubygems.new("remotes" => rubygems_sources.map(&:remotes).flatten.uniq.reverse)
+      Source::Rubygems.new("remotes" => rubygems_remotes)
     end
   end
 end
diff --git a/spec/bundler/source_list_spec.rb b/spec/bundler/source_list_spec.rb
index 67d4ef6a45..4acaa9da5d 100644
--- a/spec/bundler/source_list_spec.rb
+++ b/spec/bundler/source_list_spec.rb
@@ -277,17 +277,17 @@ describe Bundler::SourceList do
   end
 
   describe "#lock_sources" do
-    it "combines the rubygems sources into a single instance, removing duplicate remotes from the front" do
+    it "combines the rubygems sources into a single instance, removing duplicate remotes from the end" do
       source_list.add_git_source('uri' => 'git://third-git.org/path.git')
-      source_list.add_rubygems_source('remotes' => ['https://fourth-rubygems.org']) # intentional duplicate
+      source_list.add_rubygems_source('remotes' => ['https://duplicate-rubygems.org'])
       source_list.add_path_source('path' => '/third/path/to/gem')
-      source_list.add_rubygems_source('remotes' => ['https://first-rubygems.org'])
+      source_list.add_rubygems_source('remotes' => ['https://third-rubygems.org'])
       source_list.add_path_source('path' => '/second/path/to/gem')
       source_list.add_rubygems_source('remotes' => ['https://second-rubygems.org'])
       source_list.add_git_source('uri' => 'git://second-git.org/path.git')
-      source_list.add_rubygems_source('remotes' => ['https://third-rubygems.org'])
+      source_list.add_rubygems_source('remotes' => ['https://first-rubygems.org'])
       source_list.add_path_source('path' => '/first/path/to/gem')
-      source_list.add_rubygems_source('remotes' => ['https://fourth-rubygems.org'])
+      source_list.add_rubygems_source('remotes' => ['https://duplicate-rubygems.org'])
       source_list.add_git_source('uri' => 'git://first-git.org/path.git')
 
       expect(source_list.lock_sources).to eq [
@@ -298,10 +298,10 @@ describe Bundler::SourceList do
         Bundler::Source::Path.new('path' => '/second/path/to/gem'),
         Bundler::Source::Path.new('path' => '/third/path/to/gem'),
         Bundler::Source::Rubygems.new('remotes' => [
+          'https://duplicate-rubygems.org',
           'https://first-rubygems.org',
           'https://second-rubygems.org',
           'https://third-rubygems.org',
-          'https://fourth-rubygems.org',
         ]),
       ]
     end
diff --git a/spec/install/deploy_spec.rb b/spec/install/deploy_spec.rb
index c3ba3215b6..25f5c5b5de 100644
--- a/spec/install/deploy_spec.rb
+++ b/spec/install/deploy_spec.rb
@@ -79,6 +79,19 @@ describe "install with --deployment or --frozen" do
     expect(exitstatus).to eq(0)
   end
 
+  it "works with sources given by a block" do
+    install_gemfile <<-G
+      source "file://#{gem_repo1}" do
+        gem "rack"
+      end
+    G
+
+    bundle "install --deployment", :exitstatus => true
+
+    expect(exitstatus).to eq(0)
+    should_be_installed "rack 1.0"
+  end
+
   describe "with an existing lockfile" do
     before do
       bundle "install"
diff --git a/spec/install/gems/sources_spec.rb b/spec/install/gems/sources_spec.rb
index d45e25c754..546aac4583 100644
--- a/spec/install/gems/sources_spec.rb
+++ b/spec/install/gems/sources_spec.rb
@@ -83,6 +83,18 @@ describe "bundle install with gems on multiple sources" do
         expect(out).not_to include("Warning")
         should_be_installed("rack-obama 1.0.0", "rack 1.0.0")
       end
+
+      it "can cache and deploy" do
+        bundle :package
+
+        expect(bundled_app("vendor/cache/rack-1.0.0.gem")).to exist
+        expect(bundled_app("vendor/cache/rack-obama-1.0.gem")).to exist
+
+        bundle "install --deployment", :exitstatus => true
+
+        expect(exitstatus).to eq(0)
+        should_be_installed("rack-obama 1.0.0", "rack 1.0.0")
+      end
     end
 
     context "with sources set by an option" do