Merge #7075

7075: Convert binstub conflict deprecation to a warning r=indirect a=deivid-rodriguez

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

The problem was that this specific deprecation I don't think it makes sense as a deprecation.

### What was your diagnosis of the problem?

My diagnosis was that nothing is changing in bundler so that this needs to be a deprecation. This is just a potentially undesired situation that can be caused by gems shipping executables conflicting with the executables of other gems, and that can be fixed by using project specific binstubs.

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

My fix is to change this deprecation message to a warning, and to get it tested.

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

I chose this fix because it makes sense to me. With this, we can start printing this warning now without thinking of a deprecation path because I don't think we're changing anything here, really.

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

6 files changed, 65 insertions, 29 deletions

diff --git a/lib/bundler/rubygems_integration.rb b/lib/bundler/rubygems_integration.rb
index 79ff277bbe..a4519246a9 100644
--- a/lib/bundler/rubygems_integration.rb
+++ b/lib/bundler/rubygems_integration.rb
@@ -434,40 +434,42 @@ module Bundler
     # Used to make bin stubs that are not created by bundler work
     # under bundler. The new Gem.bin_path only considers gems in
     # +specs+
-    def replace_bin_path(specs, specs_by_name)
+    def replace_bin_path(specs_by_name)
       gem_class = (class << Gem; self; end)
 
       redefine_method(gem_class, :find_spec_for_exe) do |gem_name, *args|
         exec_name = args.first
+        raise ArgumentError, "you must supply exec_name" unless exec_name
 
         spec_with_name = specs_by_name[gem_name]
-        spec = if exec_name
-          if spec_with_name && spec_with_name.executables.include?(exec_name)
-            spec_with_name
-          else
-            specs.find {|s| s.executables.include?(exec_name) }
-          end
-        else
-          spec_with_name
-        end
+        matching_specs_by_exec_name = specs_by_name.values.select {|s| s.executables.include?(exec_name) }
+        spec = matching_specs_by_exec_name.delete(spec_with_name)
 
-        unless spec
+        unless spec || !matching_specs_by_exec_name.empty?
           message = "can't find executable #{exec_name} for gem #{gem_name}"
-          if !exec_name || spec_with_name.nil?
+          if spec_with_name.nil?
             message += ". #{gem_name} is not currently included in the bundle, " \
                        "perhaps you meant to add it to your #{Bundler.default_gemfile.basename}?"
           end
           raise Gem::Exception, message
         end
 
-        raise Gem::Exception, "no default executable for #{spec.full_name}" unless exec_name ||= spec.default_executable
-
-        unless spec.name == gem_name
-          Bundler::SharedHelpers.major_deprecation 2,
+        unless spec
+          spec = matching_specs_by_exec_name.shift
+          warn \
             "Bundler is using a binstub that was created for a different gem (#{spec.name}).\n" \
             "You should run `bundle binstub #{gem_name}` " \
             "to work around a system/bundle conflict."
         end
+
+        unless matching_specs_by_exec_name.empty?
+          conflicting_names = matching_specs_by_exec_name.map(&:name).join(", ")
+          warn \
+            "The `#{exec_name}` executable in the `#{spec.name}` gem is being loaded, but it's also present in other gems (#{conflicting_names}).\n" \
+            "If you meant to run the executable for another gem, make sure you use a project specific binstub (`bundle binstub <gem_name>`).\n" \
+            "If you plan to use multiple conflicting executables, generate binstubs for them and disambiguate their names."
+        end
+
         spec
       end
 
@@ -523,7 +525,7 @@ module Bundler
 
       replace_gem(specs, specs_by_name)
       stub_rubygems(specs)
-      replace_bin_path(specs, specs_by_name)
+      replace_bin_path(specs_by_name)
       replace_refresh
 
       Gem.clear_paths
diff --git a/spec/bundler/shared_helpers_spec.rb b/spec/bundler/shared_helpers_spec.rb
index 445bc18265..f42d9ed485 100644
--- a/spec/bundler/shared_helpers_spec.rb
+++ b/spec/bundler/shared_helpers_spec.rb
@@ -389,7 +389,7 @@ RSpec.describe Bundler::SharedHelpers do
 
     context "bundle executable in ENV['BUNDLE_BIN_PATH'] does not exist" do
       before { ENV["BUNDLE_BIN_PATH"] = "/does/not/exist" }
-      before { Bundler.rubygems.replace_bin_path [], [] }
+      before { Bundler.rubygems.replace_bin_path [] }
 
       it "sets BUNDLE_BIN_PATH to the bundle executable file" do
         subject.set_bundle_environment
diff --git a/spec/commands/clean_spec.rb b/spec/commands/clean_spec.rb
index cd5cb932e3..0053947c85 100644
--- a/spec/commands/clean_spec.rb
+++ b/spec/commands/clean_spec.rb
@@ -322,8 +322,6 @@ RSpec.describe "bundle clean" do
   it "does not call clean automatically when using system gems" do
     bundle! "config set path.system true"
 
-    bundle! "config list"
-
     install_gemfile! <<-G
       source "file://#{gem_repo1}"
 
@@ -331,8 +329,6 @@ RSpec.describe "bundle clean" do
       gem "rack"
     G
 
-    bundle! "info thin"
-
     install_gemfile! <<-G
       source "file://#{gem_repo1}"
 
diff --git a/spec/commands/exec_spec.rb b/spec/commands/exec_spec.rb
index 1d84669a75..a95383ee97 100644
--- a/spec/commands/exec_spec.rb
+++ b/spec/commands/exec_spec.rb
@@ -217,6 +217,35 @@ RSpec.describe "bundle exec" do
     end
   end
 
+  it "warns about executable conflicts" do
+    build_repo2 do
+      build_gem "rack_two", "1.0.0" do |s|
+        s.executables = "rackup"
+      end
+    end
+
+    bundle "config set path.system true"
+
+    install_gemfile <<-G
+      source "file://#{gem_repo1}"
+      gem "rack", "0.9.1"
+    G
+
+    Dir.chdir bundled_app2 do
+      install_gemfile bundled_app2("Gemfile"), <<-G
+        source "file://#{gem_repo2}"
+        gem "rack_two", "1.0.0"
+      G
+    end
+
+    bundle! "exec rackup"
+
+    expect(last_command.stderr).to eq(
+      "Bundler is using a binstub that was created for a different gem (rack).\n" \
+      "You should run `bundle binstub rack_two` to work around a system/bundle conflict."
+    )
+  end
+
   it "handles gems installed with --without" do
     install_gemfile <<-G, forgotten_command_line_options(:without => "middleware")
       source "file://#{gem_repo1}"
diff --git a/spec/install/binstubs_spec.rb b/spec/install/binstubs_spec.rb
index f04d3fe654..daa20693c7 100644
--- a/spec/install/binstubs_spec.rb
+++ b/spec/install/binstubs_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.describe "bundle install", :bundler => "< 3" do
+RSpec.describe "bundle install" do
   describe "when system_bindir is set" do
     # On OS X, Gem.bindir defaults to /usr/bin, so system_bindir is useful if
     # you want to avoid sudo installs for system gems with OS X's default ruby
@@ -20,7 +20,7 @@ RSpec.describe "bundle install", :bundler => "< 3" do
     end
   end
 
-  describe "when multiple gems contain the same exe", :bundler => "< 3" do
+  describe "when multiple gems contain the same exe" do
     before do
       build_repo2 do
         build_gem "fake", "14" do |s|
@@ -28,16 +28,25 @@ RSpec.describe "bundle install", :bundler => "< 3" do
         end
       end
 
-      install_gemfile <<-G, :binstubs => true
+      install_gemfile <<-G
         source "file://#{gem_repo2}"
         gem "fake"
         gem "rack"
       G
     end
 
-    it "loads the correct spec's executable" do
-      gembin("rackup")
-      expect(out).to eq("1.2")
+    it "warns about the situation" do
+      bundle! "exec rackup"
+
+      expect(last_command.stderr).to include(
+        "The `rackup` executable in the `fake` gem is being loaded, but it's also present in other gems (rack).\n" \
+        "If you meant to run the executable for another gem, make sure you use a project specific binstub (`bundle binstub <gem_name>`).\n" \
+        "If you plan to use multiple conflicting executables, generate binstubs for them and disambiguate their names."
+      ).or include(
+        "The `rackup` executable in the `rack` gem is being loaded, but it's also present in other gems (fake).\n" \
+        "If you meant to run the executable for another gem, make sure you use a project specific binstub (`bundle binstub <gem_name>`).\n" \
+        "If you plan to use multiple conflicting executables, generate binstubs for them and disambiguate their names."
+      )
     end
   end
 end
diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb
index 6470b57fb8..09e7419a98 100644
--- a/spec/support/helpers.rb
+++ b/spec/support/helpers.rb
@@ -77,7 +77,7 @@ module Spec
     def run(cmd, *args)
       opts = args.last.is_a?(Hash) ? args.pop : {}
       groups = args.map(&:inspect).join(", ")
-      setup = "require 'rubygems' ; require 'bundler' ; Bundler.setup(#{groups})\n"
+      setup = "require 'bundler' ; Bundler.setup(#{groups})\n"
       ruby(setup + cmd, opts)
     end
     bang :run