Merge #7057

7057: Review multiple sources deprecation r=indirect a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that I had not yet reviewed the deprecation about multiple global sources being present on a Gemfile, and thus the specs were skipped.

### What was your diagnosis of the problem?

My diagnosis was that we need to delay the removal of multiple sources support to bundler 3, so that we can show the deprecations in the 2.x series.

I also noticed that part of the deprecation message was inaccurate. In order to upgrade the warning to an error, you would also need to configure the `lockfile_uses_separate_rubygems_sources` setting. Otherwise you will get an error that these two settings depend on each other and can't be enabled separatedly.

### What is your fix for the problem, implemented in this PR?

My fix is to delay these feature flags to bundler 3 so that the deprecation specs pass. Also, since before giving this advice I'd like to study why we have two different settings that can't be enabled separately, and why the can't be merged to a single one, I have removed that part of the message for now.

### Why did you choose this fix out of the possible options?

I chose this fix because it keeps me moving with reviewing all the deprecations for bundler 3 breaking changes, and makes sure that the deprecations for this change of behavior are tested (and passing).

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

9 files changed, 168 insertions, 58 deletions

diff --git a/spec/bundler/source_list_spec.rb b/spec/bundler/source_list_spec.rb
index 971f1042dc..7df4ba82d3 100644
--- a/spec/bundler/source_list_spec.rb
+++ b/spec/bundler/source_list_spec.rb
@@ -372,7 +372,7 @@ RSpec.describe Bundler::SourceList do
       source_list.add_git_source("uri" => "git://first-git.org/path.git")
     end
 
-    it "combines the rubygems sources into a single instance, removing duplicate remotes from the end", :bundler => "< 2" do
+    it "combines the rubygems sources into a single instance, removing duplicate remotes from the end", :bundler => "< 3" do
       expect(source_list.lock_sources).to eq [
         Bundler::Source::Git.new("uri" => "git://first-git.org/path.git"),
         Bundler::Source::Git.new("uri" => "git://second-git.org/path.git"),
@@ -391,7 +391,7 @@ RSpec.describe Bundler::SourceList do
       ]
     end
 
-    it "returns all sources, without combining rubygems sources", :bundler => "2" do
+    it "returns all sources, without combining rubygems sources", :bundler => "3" do
       expect(source_list.lock_sources).to eq [
         Bundler::Source::Git.new("uri" => "git://first-git.org/path.git"),
         Bundler::Source::Git.new("uri" => "git://second-git.org/path.git"),
diff --git a/spec/commands/exec_spec.rb b/spec/commands/exec_spec.rb
index 35b5eaea00..bf1de25142 100644
--- a/spec/commands/exec_spec.rb
+++ b/spec/commands/exec_spec.rb
@@ -693,6 +693,7 @@ RSpec.describe "bundle exec" do
       end
 
       let(:exit_code) { Bundler::GemNotFound.new.status_code }
+      let(:expected_err) { "" }
       let(:expected) { <<-EOS.strip }
 \e[31mCould not find gem 'rack (= 2)' in any of the gem sources listed in your Gemfile.\e[0m
 \e[33mRun `bundle install` to install missing gems.\e[0m
@@ -712,6 +713,24 @@ RSpec.describe "bundle exec" do
       let(:exit_code) { Bundler::GemNotFound.new.status_code }
       let(:expected) { "" }
       let(:expected_err) { <<-EOS.strip }
+\e[31mCould not find gem 'rack (= 2)' in any of the gem sources listed in your Gemfile.\e[0m
+\e[33mRun `bundle install` to install missing gems.\e[0m
+      EOS
+
+      it_behaves_like "it runs"
+    end
+
+    context "when Bundler.setup fails", :bundler => "3" do
+      before do
+        gemfile <<-G
+          gem 'rack', '2'
+        G
+        ENV["BUNDLER_FORCE_TTY"] = "true"
+      end
+
+      let(:exit_code) { Bundler::GemNotFound.new.status_code }
+      let(:expected) { "" }
+      let(:expected_err) { <<-EOS.strip }
 \e[31mCould not find gem 'rack (= 2)' in locally installed gems.
 The source contains 'rack' at: 1.0.0\e[0m
 \e[33mRun `bundle install` to install missing gems.\e[0m
diff --git a/spec/commands/list_spec.rb b/spec/commands/list_spec.rb
index 613249cc59..e88d7bfb04 100644
--- a/spec/commands/list_spec.rb
+++ b/spec/commands/list_spec.rb
@@ -1,15 +1,6 @@
 # frozen_string_literal: true
 
 RSpec.describe "bundle list", :bundler => ">= 2" do
-  before do
-    install_gemfile <<-G
-      source "file://#{gem_repo1}"
-
-      gem "rack"
-      gem "rspec", :group => [:test]
-    G
-  end
-
   context "with name-only and paths option" do
     it "raises an error" do
       bundle "list --name-only --paths"
@@ -27,6 +18,15 @@ RSpec.describe "bundle list", :bundler => ">= 2" do
   end
 
   describe "with without-group option" do
+    before do
+      install_gemfile <<-G
+        source "file://#{gem_repo1}"
+
+        gem "rack"
+        gem "rspec", :group => [:test]
+      G
+    end
+
     context "when group is present" do
       it "prints the gems not in the specified group" do
         bundle! "list --without-group test"
@@ -46,6 +46,15 @@ RSpec.describe "bundle list", :bundler => ">= 2" do
   end
 
   describe "with only-group option" do
+    before do
+      install_gemfile <<-G
+        source "file://#{gem_repo1}"
+
+        gem "rack"
+        gem "rspec", :group => [:test]
+      G
+    end
+
     context "when group is present" do
       it "prints the gems in the specified group" do
         bundle! "list --only-group default"
@@ -65,6 +74,15 @@ RSpec.describe "bundle list", :bundler => ">= 2" do
   end
 
   context "with name-only option" do
+    before do
+      install_gemfile <<-G
+        source "file://#{gem_repo1}"
+
+        gem "rack"
+        gem "rspec", :group => [:test]
+      G
+    end
+
     it "prints only the name of the gems in the bundle" do
       bundle "list --name-only"
 
@@ -82,7 +100,6 @@ RSpec.describe "bundle list", :bundler => ">= 2" do
       build_git "git_test", "1.0.0", :path => lib_path("git_test")
 
       build_lib("gemspec_test", :path => tmp.join("gemspec_test")) do |s|
-        s.write("Gemfile", "source :rubygems\ngemspec")
         s.add_dependency "bar", "=1.0.0"
       end
 
@@ -93,8 +110,6 @@ RSpec.describe "bundle list", :bundler => ">= 2" do
         gem "git_test", :git => "#{lib_path("git_test")}"
         gemspec :path => "#{tmp.join("gemspec_test")}"
       G
-
-      bundle! "install"
     end
 
     it "prints the path of each gem in the bundle" do
@@ -119,13 +134,35 @@ RSpec.describe "bundle list", :bundler => ">= 2" do
     end
   end
 
-  it "lists gems installed in the bundle" do
-    bundle "list"
-    expect(out).to include("  * rack (1.0.0)")
+  context "without options" do
+    before do
+      install_gemfile <<-G
+        source "file://#{gem_repo1}"
+
+        gem "rack"
+        gem "rspec", :group => [:test]
+      G
+    end
+
+    it "lists gems installed in the bundle" do
+      bundle "list"
+      expect(out).to include("  * rack (1.0.0)")
+    end
   end
 
-  it "aliases the ls command to list" do
-    bundle "ls"
-    expect(out).to include("Gems included by the bundle")
+  context "when using the ls alias" do
+    before do
+      install_gemfile <<-G
+        source "file://#{gem_repo1}"
+
+        gem "rack"
+        gem "rspec", :group => [:test]
+      G
+    end
+
+    it "runs the list command" do
+      bundle "ls"
+      expect(out).to include("Gems included by the bundle")
+    end
   end
 end
diff --git a/spec/install/gemfile/sources_spec.rb b/spec/install/gemfile/sources_spec.rb
index b0de8a1f20..8e1205dfa3 100644
--- a/spec/install/gemfile/sources_spec.rb
+++ b/spec/install/gemfile/sources_spec.rb
@@ -15,7 +15,7 @@ RSpec.describe "bundle install with gems on multiple sources" do
       end
     end
 
-    context "with multiple toplevel sources", :bundler => "< 2" do
+    context "with multiple toplevel sources", :bundler => "< 3" do
       let(:repo3_rack_version) { "1.0.0" }
 
       before do
@@ -27,13 +27,7 @@ RSpec.describe "bundle install with gems on multiple sources" do
         G
       end
 
-      xit "shows a deprecation" do
-        bundle :install
-
-        expect(deprecations).to include("Your Gemfile contains multiple primary sources.")
-      end
-
-      it "warns about ambiguous gems, but installs anyway, prioritizing sources last to first" do
+      it "warns about ambiguous gems, but installs anyway, prioritizing sources last to first", :bundler => "2" do
         bundle :install
 
         expect(err).to include("Warning: the gem 'rack' was found in multiple sources.")
@@ -41,15 +35,14 @@ RSpec.describe "bundle install with gems on multiple sources" do
         expect(the_bundle).to include_gems("rack-obama 1.0.0", "rack 1.0.0", :source => "remote1")
       end
 
-      it "errors when disable_multisource is set" do
-        bundle "config set disable_multisource true"
+      it "fails", :bundler => "3" do
         bundle :install
         expect(err).to include("Each source after the first must include a block")
         expect(exitstatus).to eq(4) if exitstatus
       end
     end
 
-    context "when different versions of the same gem are in multiple sources", :bundler => "< 2" do
+    context "when different versions of the same gem are in multiple sources", :bundler => "< 3" do
       let(:repo3_rack_version) { "1.2" }
 
       before do
@@ -63,15 +56,16 @@ RSpec.describe "bundle install with gems on multiple sources" do
         bundle :install
       end
 
-      xit "shows a deprecation" do
-        expect(deprecations).to include("Your Gemfile contains multiple primary sources.")
-      end
-
-      it "warns about ambiguous gems, but installs anyway" do
+      it "warns about ambiguous gems, but installs anyway", :bundler => "2" do
         expect(err).to include("Warning: the gem 'rack' was found in multiple sources.")
         expect(err).to include(normalize_uri_file("Installed from: file://localhost#{gem_repo1}"))
         expect(the_bundle).to include_gems("rack-obama 1.0.0", "rack 1.0.0", :source => "remote1")
       end
+
+      it "fails", :bundler => "3" do
+        expect(err).to include("Each source after the first must include a block")
+        expect(exitstatus).to eq(4) if exitstatus
+      end
     end
   end
 
@@ -193,9 +187,8 @@ RSpec.describe "bundle install with gems on multiple sources" do
             end
           end
 
-          context "when lockfile_uses_separate_rubygems_sources is set" do
+          context "when disable_multisource is set" do
             before do
-              bundle! "config set lockfile_uses_separate_rubygems_sources true"
               bundle! "config set disable_multisource true"
             end
 
@@ -243,7 +236,7 @@ RSpec.describe "bundle install with gems on multiple sources" do
           end
         end
 
-        context "and in yet another source", :bundler => "< 2" do
+        context "and in yet another source", :bundler => "< 3" do
           before do
             gemfile <<-G
               source "file://localhost#{gem_repo1}"
@@ -256,18 +249,19 @@ RSpec.describe "bundle install with gems on multiple sources" do
             bundle :install
           end
 
-          xit "shows a deprecation" do
-            expect(deprecations).to include("Your Gemfile contains multiple primary sources.")
-          end
-
-          it "installs from the other source and warns about ambiguous gems" do
+          it "installs from the other source and warns about ambiguous gems", :bundler => "2" do
             expect(err).to include("Warning: the gem 'rack' was found in multiple sources.")
             expect(err).to include(normalize_uri_file("Installed from: file://localhost#{gem_repo2}"))
             expect(the_bundle).to include_gems("depends_on_rack 1.0.1", "rack 1.0.0")
           end
+
+          it "fails", :bundler => "3" do
+            expect(err).to include("Each source after the first must include a block")
+            expect(exitstatus).to eq(4) if exitstatus
+          end
         end
 
-        context "and only the dependency is pinned", :bundler => "< 2" do
+        context "and only the dependency is pinned", :bundler => "< 3" do
           before do
             # need this to be broken to check for correct source ordering
             build_repo gem_repo2 do
@@ -285,7 +279,7 @@ RSpec.describe "bundle install with gems on multiple sources" do
             G
           end
 
-          it "installs the dependency from the pinned source without warning" do
+          it "installs the dependency from the pinned source without warning", :bundler => "2" do
             bundle :install
 
             expect(err).not_to include("Warning: the gem 'rack' was found in multiple sources.")
@@ -299,14 +293,19 @@ RSpec.describe "bundle install with gems on multiple sources" do
             expect(err).not_to include("Warning: the gem 'rack' was found in multiple sources.")
             expect(the_bundle).to include_gems("depends_on_rack 1.0.1", "rack 1.0.0")
           end
+
+          it "fails", :bundler => "3" do
+            bundle :install
+            expect(err).to include("Each source after the first must include a block")
+            expect(exitstatus).to eq(4) if exitstatus
+          end
         end
       end
     end
 
     context "when a top-level gem has an indirect dependency" do
-      context "when lockfile_uses_separate_rubygems_sources is set" do
+      context "when disable_multisource is set" do
         before do
-          bundle! "config set lockfile_uses_separate_rubygems_sources true"
           bundle! "config set disable_multisource true"
         end
 
@@ -626,7 +625,39 @@ RSpec.describe "bundle install with gems on multiple sources" do
     end
   end
 
-  context "when a gem is available from multiple ambiguous sources", :bundler => "2" do
+  describe "source changed to one containing a higher version of a dependency" do
+    before do
+      install_gemfile! <<-G
+        source "file://#{gem_repo1}"
+
+        gem "rack"
+      G
+
+      build_repo2 do
+        build_gem "bar"
+      end
+
+      build_lib("gemspec_test", :path => tmp.join("gemspec_test")) do |s|
+        s.add_dependency "bar", "=1.0.0"
+      end
+
+      install_gemfile <<-G
+        source "file://#{gem_repo2}"
+        gem "rack"
+        gemspec :path => "#{tmp.join("gemspec_test")}"
+      G
+    end
+
+    it "keeps the old version", :bundler => "2" do
+      expect(the_bundle).to include_gems("rack 1.0.0")
+    end
+
+    it "installs the higher version in the new repo", :bundler => "3" do
+      expect(the_bundle).to include_gems("rack 1.2")
+    end
+  end
+
+  context "when a gem is available from multiple ambiguous sources", :bundler => "3" do
     it "raises, suggesting a source block" do
       build_repo4 do
         build_gem "depends_on_rack" do |s|
diff --git a/spec/install/gems/flex_spec.rb b/spec/install/gems/flex_spec.rb
index 12bc2877ff..aeb83d6573 100644
--- a/spec/install/gems/flex_spec.rb
+++ b/spec/install/gems/flex_spec.rb
@@ -244,7 +244,7 @@ RSpec.describe "bundle flex_install" do
   end
 
   describe "when adding a new source" do
-    it "updates the lockfile", :bundler => "< 2" do
+    it "updates the lockfile", :bundler => "< 3" do
       build_repo2
       install_gemfile! <<-G
         source "file://localhost#{gem_repo1}"
@@ -264,7 +264,7 @@ RSpec.describe "bundle flex_install" do
           rack (1.0.0)
 
       PLATFORMS
-        ruby
+        #{lockfile_platforms}
 
       DEPENDENCIES
         rack
@@ -274,7 +274,7 @@ RSpec.describe "bundle flex_install" do
       L
     end
 
-    it "updates the lockfile", :bundler => "2" do
+    it "updates the lockfile", :bundler => "3" do
       build_repo2
       install_gemfile! <<-G
         source "file://localhost#{gem_repo1}"
diff --git a/spec/install/post_bundle_message_spec.rb b/spec/install/post_bundle_message_spec.rb
index 602373896d..1ea21a6635 100644
--- a/spec/install/post_bundle_message_spec.rb
+++ b/spec/install/post_bundle_message_spec.rb
@@ -101,16 +101,16 @@ RSpec.describe "post bundle message" do
     end
 
     describe "with misspelled or non-existent gem name" do
-      it "should report a helpful error message", :bundler => "< 2" do
+      it "should report a helpful error message", :bundler => "< 3" do
         install_gemfile <<-G
           source "file://localhost#{gem_repo1}"
           gem "rack"
           gem "not-a-gem", :group => :development
         G
-        expect(out).to include("Could not find gem 'not-a-gem' in any of the gem sources listed in your Gemfile.")
+        expect(err).to include("Could not find gem 'not-a-gem' in any of the gem sources listed in your Gemfile.")
       end
 
-      it "should report a helpful error message", :bundler => "2" do
+      it "should report a helpful error message", :bundler => "3" do
         install_gemfile <<-G
           source "file://localhost#{gem_repo1}"
           gem "rack"
diff --git a/spec/lock/lockfile_spec.rb b/spec/lock/lockfile_spec.rb
index c07c4e1db8..0bab70066e 100644
--- a/spec/lock/lockfile_spec.rb
+++ b/spec/lock/lockfile_spec.rb
@@ -415,7 +415,7 @@ RSpec.describe "the lockfile format" do
     G
   end
 
-  it "generates a lockfile without credentials for a configured source", :bundler => "< 2" do
+  it "generates a lockfile without credentials for a configured source", :bundler => "< 3" do
     bundle "config set http://localgemserver.test/ user:pass"
 
     install_gemfile(<<-G, :artifice => "endpoint_strict_basic_authentication", :quiet => true)
@@ -448,7 +448,7 @@ RSpec.describe "the lockfile format" do
     G
   end
 
-  it "generates a lockfile without credentials for a configured source", :bundler => "2" do
+  it "generates a lockfile without credentials for a configured source", :bundler => "3" do
     bundle "config set http://localgemserver.test/ user:pass"
 
     install_gemfile(<<-G, :artifice => "endpoint_strict_basic_authentication", :quiet => true)
diff --git a/spec/other/major_deprecation_spec.rb b/spec/other/major_deprecation_spec.rb
index d041fd13bd..cd8b721609 100644
--- a/spec/other/major_deprecation_spec.rb
+++ b/spec/other/major_deprecation_spec.rb
@@ -345,6 +345,30 @@ RSpec.describe "major deprecations" do
     end
   end
 
+  context "bundle install with multiple sources" do
+    before do
+      install_gemfile <<-G
+        source "file://localhost#{gem_repo3}"
+        source "file://localhost#{gem_repo1}"
+      G
+    end
+
+    it "does not print a deprecation warning", :bundler => "< 2" do
+      expect(deprecations).to be_empty
+    end
+
+    it "shows a deprecation", :bundler => "2" do
+      expect(deprecations).to include(
+        "Your Gemfile contains multiple primary sources. " \
+        "Using `source` more than once without a block is a security risk, and " \
+        "may result in installing unexpected gems. To resolve this warning, use " \
+        "a block to indicate which gems should come from the secondary source. " \
+        "To upgrade this warning to an error, run `bundle config set " \
+        "disable_multisource true`."
+      )
+    end
+  end
+
   context "when Bundler.setup is run in a ruby script" do
     before do
       create_file "gems.rb"
diff --git a/spec/quality_spec.rb b/spec/quality_spec.rb
index c9df900647..f9c815aee1 100644
--- a/spec/quality_spec.rb
+++ b/spec/quality_spec.rb
@@ -173,7 +173,6 @@ RSpec.describe "The library itself" do
       gem.mit
       github.https
       inline
-      lockfile_uses_separate_rubygems_sources
       use_gem_version_promoter_for_major_updates
     ]