Merge #7401

7401: Stop silencing output by default r=indirect a=deivid-rodriguez

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

The problem was that bundler defaults to a silent UI:

https://github.com/bundler/bundler/blob/e70643c1be3a4417bd537d7e63470265465e693e/lib/bundler.rb#L66-L68

In my opinion, this isn't a good behavior for a CLI tool, and forces us to override it in many many different places. It has also caused several issues, for example, https://github.com/bundler/bundler/issues/3710 where `bundle list` was printing nothing.

The [solution to that issues](https://github.com/bundler/bundler/pull/3707) led us to add yet more places where we override the default UI, and @indirect [predicting that having to unset the UI everytime we want it to not be silent](https://github.com/bundler/bundler/pull/3707#issuecomment-108646127) would cause many headaches.

Well, yeah... I've lost a lot of time trying to figure out why UI was silent sometimes, and normal another times, why some specs printed warnings and some didn't. In particular, see my series of "big fail PRs" fighting against bundler's UI: https://github.com/bundler/bundler/pull/7284, https://github.com/bundler/bundler/pull/7294, https://github.com/bundler/bundler/pull/7305, https://github.com/bundler/bundler/pull/7362.

Another series of issues/PRs probably related to this is issue https://github.com/bundler/bundler/issues/6900, where the output would use a different UI on different environments. We had a lot of trouble to reliably fix it (https://github.com/bundler/bundler/pull/6994, https://github.com/bundler/bundler/pull/7002, https://github.com/bundler/bundler/issues/7253).

I also run into these issues again when trying out the `RUBYGEMS_GEMDEPS` environment variable that enables `bundler/setup` from rubygems.

### What was your diagnosis of the problem?

My diagnosis was that we shouldn't silence UI by default.

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

My fix is to, instead of silencing and then overriding the default shell at many places, don't silence it by default and instead make it silent when needed. By doing this, I managed to get 100% of our specs green, so I'm pretty confident that the output is still the same (or if it's not, it's probably because some output/errors where being unintentionally swallowed). 

Now specs should pass, but they print a bunch of output to the screen. You can see error messages, hard crashes, success messages... Some of them might be showing actual issues with either the code or tests, so I plan to go through each of them and review them. I can do that in this PR or separately, no strong opinion.

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

18 files changed, 54 insertions, 76 deletions

diff --git a/lib/bundler.rb b/lib/bundler.rb
index 4321a7ed3d..2ada6fe789 100644
--- a/lib/bundler.rb
+++ b/lib/bundler.rb
@@ -64,11 +64,11 @@ module Bundler
     end
 
     def ui
-      (defined?(@ui) && @ui) || (self.ui = UI::Silent.new)
+      (defined?(@ui) && @ui) || (self.ui = UI::Shell.new)
     end
 
     def ui=(ui)
-      Bundler.rubygems.ui = ui ? UI::RGProxy.new(ui) : nil
+      Bundler.rubygems.ui = UI::RGProxy.new(ui)
       @ui = ui
     end
 
diff --git a/lib/bundler/cli.rb b/lib/bundler/cli.rb
index bacbb8dbde..d7f749a672 100644
--- a/lib/bundler/cli.rb
+++ b/lib/bundler/cli.rb
@@ -22,9 +22,6 @@ module Bundler
 
     def self.start(*)
       super
-    rescue Exception => e # rubocop:disable Lint/RescueException
-      Bundler.ui = UI::Shell.new
-      raise e
     ensure
       Bundler::SharedHelpers.print_major_deprecations!
     end
diff --git a/lib/bundler/cli/exec.rb b/lib/bundler/cli/exec.rb
index 0b0e991ea5..0a1edbdbbd 100644
--- a/lib/bundler/cli/exec.rb
+++ b/lib/bundler/cli/exec.rb
@@ -43,15 +43,11 @@ module Bundler
     end
 
     def kernel_exec(*args)
-      ui = Bundler.ui
-      Bundler.ui = nil
       Kernel.exec(*args)
     rescue Errno::EACCES, Errno::ENOEXEC
-      Bundler.ui = ui
       Bundler.ui.error "bundler: not executable: #{cmd}"
       exit 126
     rescue Errno::ENOENT
-      Bundler.ui = ui
       Bundler.ui.error "bundler: command not found: #{cmd}"
       Bundler.ui.warn "Install missing gem executables with `bundle install`"
       exit 127
@@ -62,15 +58,12 @@ module Bundler
       ARGV.replace(args)
       $0 = file
       Process.setproctitle(process_title(file, args)) if Process.respond_to?(:setproctitle)
-      ui = Bundler.ui
-      Bundler.ui = nil
       require_relative "../setup"
       TRAPPED_SIGNALS.each {|s| trap(s, "DEFAULT") }
       Kernel.load(file)
     rescue SystemExit, SignalException
       raise
     rescue Exception => e # rubocop:disable Lint/RescueException
-      Bundler.ui = ui
       Bundler.ui.error "bundler: failed to load command: #{cmd} (#{file})"
       backtrace = e.backtrace ? e.backtrace.take_while {|bt| !bt.start_with?(__FILE__) } : []
       abort "#{e.class}: #{e.message}\n  #{backtrace.join("\n  ")}"
diff --git a/lib/bundler/friendly_errors.rb b/lib/bundler/friendly_errors.rb
index e9f06d90be..273573e820 100644
--- a/lib/bundler/friendly_errors.rb
+++ b/lib/bundler/friendly_errors.rb
@@ -16,7 +16,7 @@ module Bundler
         Bundler.ui.error error.message
       when GemRequireError
         Bundler.ui.error error.message
-        Bundler.ui.trace error.orig_exception, nil, true
+        Bundler.ui.trace error.orig_exception
       when BundlerError
         Bundler.ui.error error.message, :wrap => true
         Bundler.ui.trace error
diff --git a/lib/bundler/gem_helper.rb b/lib/bundler/gem_helper.rb
index d00c1894a8..81872b9429 100644
--- a/lib/bundler/gem_helper.rb
+++ b/lib/bundler/gem_helper.rb
@@ -26,7 +26,6 @@ module Bundler
     attr_reader :spec_path, :base, :gemspec
 
     def initialize(base = nil, name = nil)
-      Bundler.ui = UI::Shell.new
       @base = (base ||= SharedHelpers.pwd)
       gemspecs = name ? [File.join(base, "#{name}.gemspec")] : Dir[File.join(base, "{,*}.gemspec")]
       raise "Unable to determine name from existing gemspec. Use :name => 'gemname' in #install_tasks to manually set it." unless gemspecs.size == 1
diff --git a/lib/bundler/inline.rb b/lib/bundler/inline.rb
index dbf737c7ee..5491555d67 100644
--- a/lib/bundler/inline.rb
+++ b/lib/bundler/inline.rb
@@ -56,7 +56,7 @@ def gemfile(install = false, options = {}, &gemfile)
       definition.missing_specs?
     end
 
-    Bundler.ui = ui if install
+    Bundler.ui = install ? ui : Bundler::UI::Silent.new
     if install || missing_specs.call
       Bundler.settings.temporary(:inline => true, :disable_platform_warnings => true) do
         installer = Bundler::Installer.install(Bundler.root, definition, :system => true)
diff --git a/lib/bundler/rubygems_integration.rb b/lib/bundler/rubygems_integration.rb
index 7549c592f1..c4950d14e8 100644
--- a/lib/bundler/rubygems_integration.rb
+++ b/lib/bundler/rubygems_integration.rb
@@ -635,7 +635,6 @@ module Bundler
       ENV["BUNDLE_GEMFILE"] ||= File.expand_path(gemfile)
       require_relative "gemdeps"
       runtime = Bundler.setup
-      Bundler.ui = nil
       activated_spec_names = runtime.requested_specs.map(&:to_spec).sort_by(&:name)
       [Gemdeps.new(runtime), activated_spec_names]
     end
diff --git a/lib/bundler/setup.rb b/lib/bundler/setup.rb
index d156f494a8..b9a2f8f1da 100644
--- a/lib/bundler/setup.rb
+++ b/lib/bundler/setup.rb
@@ -6,9 +6,8 @@ if Bundler::SharedHelpers.in_bundle?
   require_relative "../bundler"
 
   if STDOUT.tty? || ENV["BUNDLER_FORCE_TTY"]
-    Bundler.ui = Bundler::UI::Shell.new
     begin
-      Bundler.setup
+      Bundler.ui.silence { Bundler.setup }
     rescue Bundler::BundlerError => e
       Bundler.ui.warn "\e[31m#{e.message}\e[0m"
       Bundler.ui.warn e.backtrace.join("\n") if ENV["DEBUG"]
@@ -18,12 +17,10 @@ if Bundler::SharedHelpers.in_bundle?
       exit e.status_code
     end
   else
-    Bundler.setup
+    Bundler.ui.silence { Bundler.setup }
   end
 
   # Add bundler to the load path after disabling system gems
   bundler_lib = File.expand_path("../..", __FILE__)
   $LOAD_PATH.unshift(bundler_lib) unless $LOAD_PATH.include?(bundler_lib)
-
-  Bundler.ui = nil
 end
diff --git a/lib/bundler/shared_helpers.rb b/lib/bundler/shared_helpers.rb
index dec03ed160..6e83bc5ff4 100644
--- a/lib/bundler/shared_helpers.rb
+++ b/lib/bundler/shared_helpers.rb
@@ -137,10 +137,7 @@ module Bundler
       end
 
       return unless bundler_major_version >= major_version && prints_major_deprecations?
-      @major_deprecation_ui ||= Bundler::UI::Shell.new("no-color" => true)
-      with_major_deprecation_ui do |ui|
-        ui.warn("[DEPRECATED] #{message}")
-      end
+      Bundler.ui.warn("[DEPRECATED] #{message}")
     end
 
     def print_major_deprecations!
@@ -217,21 +214,6 @@ module Bundler
 
   private
 
-    def with_major_deprecation_ui(&block)
-      ui = Bundler.ui
-
-      if ui.is_a?(@major_deprecation_ui.class)
-        yield ui
-      else
-        begin
-          Bundler.ui = @major_deprecation_ui
-          yield Bundler.ui
-        ensure
-          Bundler.ui = ui
-        end
-      end
-    end
-
     def validate_bundle_path
       path_separator = Bundler.rubygems.path_separator
       return unless Bundler.bundle_path.to_s.split(path_separator).size > 1
diff --git a/spec/bundler/friendly_errors_spec.rb b/spec/bundler/friendly_errors_spec.rb
index 858831c448..47e7a5d3cd 100644
--- a/spec/bundler/friendly_errors_spec.rb
+++ b/spec/bundler/friendly_errors_spec.rb
@@ -94,7 +94,7 @@ RSpec.describe Bundler, "friendly errors" do
       end
 
       it "writes to Bundler.ui.trace" do
-        expect(Bundler.ui).to receive(:trace).with(orig_error, nil, true)
+        expect(Bundler.ui).to receive(:trace).with(orig_error)
         Bundler::FriendlyErrors.log_error(error)
       end
     end
diff --git a/spec/bundler/source_spec.rb b/spec/bundler/source_spec.rb
index 5b11503d23..a687d8a565 100644
--- a/spec/bundler/source_spec.rb
+++ b/spec/bundler/source_spec.rb
@@ -56,10 +56,9 @@ RSpec.describe Bundler::Source do
         context "with a different version" do
           let(:locked_gem) { double(:locked_gem, :name => "nokogiri", :version => "< 1.5") }
 
-          context "with color", :no_color_tty do
+          context "with color" do
             before do
               allow($stdout).to receive(:tty?).and_return(true)
-              Bundler.ui = Bundler::UI::Shell.new
             end
 
             it "should return a string with the spec name and version and locked spec version" do
@@ -68,7 +67,11 @@ RSpec.describe Bundler::Source do
           end
 
           context "without color" do
-            before { Bundler.ui = Bundler::UI::Shell.new("no-color" => true) }
+            around do |example|
+              with_ui(Bundler::UI::Shell.new("no-color" => true)) do
+                example.run
+              end
+            end
 
             it "should return a string with the spec name and version and locked spec version" do
               expect(subject.version_message(spec)).to eq("nokogiri >= 1.6 (was < 1.5)")
@@ -80,10 +83,9 @@ RSpec.describe Bundler::Source do
           let(:spec) { double(:spec, :name => "nokogiri", :version => "1.6.1", :platform => rb) }
           let(:locked_gem) { double(:locked_gem, :name => "nokogiri", :version => "1.7.0") }
 
-          context "with color", :no_color_tty do
+          context "with color" do
             before do
               allow($stdout).to receive(:tty?).and_return(true)
-              Bundler.ui = Bundler::UI::Shell.new
             end
 
             it "should return a string with the locked spec version in yellow" do
@@ -92,7 +94,11 @@ RSpec.describe Bundler::Source do
           end
 
           context "without color" do
-            before { Bundler.ui = Bundler::UI::Shell.new("no-color" => true) }
+            around do |example|
+              with_ui(Bundler::UI::Shell.new("no-color" => true)) do
+                example.run
+              end
+            end
 
             it "should return a string with the locked spec version in yellow" do
               expect(subject.version_message(spec)).to eq("nokogiri 1.6.1 (was 1.7.0)")
@@ -104,10 +110,9 @@ RSpec.describe Bundler::Source do
           let(:spec) { double(:spec, :name => "nokogiri", :version => "1.7.1", :platform => rb) }
           let(:locked_gem) { double(:locked_gem, :name => "nokogiri", :version => "1.7.0") }
 
-          context "with color", :no_color_tty do
+          context "with color" do
             before do
               allow($stdout).to receive(:tty?).and_return(true)
-              Bundler.ui = Bundler::UI::Shell.new
             end
 
             it "should return a string with the locked spec version in green" do
@@ -116,7 +121,11 @@ RSpec.describe Bundler::Source do
           end
 
           context "without color" do
-            before { Bundler.ui = Bundler::UI::Shell.new("no-color" => true) }
+            around do |example|
+              with_ui(Bundler::UI::Shell.new("no-color" => true)) do
+                example.run
+              end
+            end
 
             it "should return a string with the locked spec version in yellow" do
               expect(subject.version_message(spec)).to eq("nokogiri 1.7.1 (was 1.7.0)")
@@ -178,4 +187,14 @@ RSpec.describe Bundler::Source do
       end
     end
   end
+
+private
+
+  def with_ui(ui)
+    old_ui = Bundler.ui
+    Bundler.ui = ui
+    yield
+  ensure
+    Bundler.ui = old_ui
+  end
 end
diff --git a/spec/other/major_deprecation_spec.rb b/spec/other/major_deprecation_spec.rb
index 52fb532a4a..57b68fdb97 100644
--- a/spec/other/major_deprecation_spec.rb
+++ b/spec/other/major_deprecation_spec.rb
@@ -360,7 +360,6 @@ RSpec.describe "major deprecations" do
         require 'bundler'
         require 'bundler/vendored_thor'
 
-        Bundler.ui = Bundler::UI::Shell.new
         Bundler.setup
         Bundler.setup
       RUBY
diff --git a/spec/realworld/edgecases_spec.rb b/spec/realworld/edgecases_spec.rb
index 6468ee7f1e..26dd096b28 100644
--- a/spec/realworld/edgecases_spec.rb
+++ b/spec/realworld/edgecases_spec.rb
@@ -7,10 +7,12 @@ RSpec.describe "real world edgecases", :realworld => true, :sometimes => true do
       require "bundler"
       require "bundler/source/rubygems/remote"
       require "bundler/fetcher"
-      source = Bundler::Source::Rubygems::Remote.new(URI("https://rubygems.org"))
-      fetcher = Bundler::Fetcher.new(source)
-      index = fetcher.specs([#{name.dump}], nil)
-      rubygem = index.search(Gem::Dependency.new(#{name.dump}, #{requirement.dump})).last
+      rubygem = Bundler.ui.silence do
+        source = Bundler::Source::Rubygems::Remote.new(URI("https://rubygems.org"))
+        fetcher = Bundler::Fetcher.new(source)
+        index = fetcher.specs([#{name.dump}], nil)
+        index.search(Gem::Dependency.new(#{name.dump}, #{requirement.dump})).last
+      end
       if rubygem.nil?
         raise "Could not find #{name} (#{requirement}) on rubygems.org!\n" \
           "Found specs:\n\#{index.send(:specs).inspect}"
diff --git a/spec/runtime/platform_spec.rb b/spec/runtime/platform_spec.rb
index c504685ea3..f7e93eacf1 100644
--- a/spec/runtime/platform_spec.rb
+++ b/spec/runtime/platform_spec.rb
@@ -23,7 +23,7 @@ RSpec.describe "Bundler.setup with multi platform stuff" do
     ruby <<-R
       begin
         require 'bundler'
-        Bundler.setup
+        Bundler.ui.silence { Bundler.setup }
       rescue Bundler::GemNotFound => e
         puts "WIN"
       end
diff --git a/spec/runtime/with_unbundled_env_spec.rb b/spec/runtime/with_unbundled_env_spec.rb
index 05f3c8e3ab..254f493042 100644
--- a/spec/runtime/with_unbundled_env_spec.rb
+++ b/spec/runtime/with_unbundled_env_spec.rb
@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require_relative "../support/streams"
-
 RSpec.describe "Bundler.with_env helpers" do
   def bundle_exec_ruby!(code, options = {})
     build_bundler_context options
@@ -129,16 +127,16 @@ RSpec.describe "Bundler.with_env helpers" do
   describe "Bundler.with_clean_env", :bundler => 2 do
     it "should set ENV to unbundled_env in the block" do
       expected = Bundler.unbundled_env
-      actual = nil
-      capture(:stderr) do
-        actual = Bundler.with_clean_env { ENV.to_hash }
+
+      actual = Bundler.ui.silence do
+        Bundler.with_clean_env { ENV.to_hash }
       end
 
       expect(actual).to eq(expected)
     end
 
     it "should restore the environment after execution" do
-      capture(:stderr) do
+      Bundler.ui.silence do
         Bundler.with_clean_env { ENV["FOO"] = "hello" }
       end
 
@@ -181,9 +179,7 @@ RSpec.describe "Bundler.with_env helpers" do
   describe "Bundler.clean_system", :bundler => 2 do
     let(:code) do
       <<~RUBY
-        capture(:stderr) do
-          Bundler.clean_system(%([ "\$BUNDLE_FOO" = "bar" ] || exit 42))
-        end
+        Bundler.ui.silence { Bundler.clean_system(%([ "\$BUNDLE_FOO" = "bar" ] || exit 42)) }
 
         exit $?.exitstatus
       RUBY
@@ -191,7 +187,7 @@ RSpec.describe "Bundler.with_env helpers" do
 
     it "runs system inside with_clean_env" do
       lib = File.expand_path("../../lib", __dir__)
-      system({ "BUNDLE_FOO" => "bar" }, "ruby -I#{lib}:#{spec} -rsupport/streams -rbundler -e '#{code}'")
+      system({ "BUNDLE_FOO" => "bar" }, "ruby -I#{lib} -rbundler -e '#{code}'")
       expect($?.exitstatus).to eq(42)
     end
   end
@@ -237,10 +233,8 @@ RSpec.describe "Bundler.with_env helpers" do
   describe "Bundler.clean_exec", :bundler => 2 do
     let(:code) do
       <<~RUBY
-        capture(:stderr) do
-          Process.fork do
-            exit Bundler.clean_exec(%(test "\$BUNDLE_FOO" = "bar"))
-          end
+        Process.fork do
+          exit Bundler.ui.silence { Bundler.clean_exec(%(test "\$BUNDLE_FOO" = "bar")) }
         end
 
         _, status = Process.wait2
@@ -253,7 +247,7 @@ RSpec.describe "Bundler.with_env helpers" do
       skip "Fork not implemented" if Gem.win_platform?
 
       lib = File.expand_path("../../lib", __dir__)
-      system({ "BUNDLE_FOO" => "bar" }, "ruby -I#{lib}:#{spec} -rsupport/streams -rbundler -e '#{code}'")
+      system({ "BUNDLE_FOO" => "bar" }, "ruby -I#{lib} -rbundler -e '#{code}'")
       expect($?.exitstatus).to eq(1)
     end
   end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 34982c8ca8..98bc21e537 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -107,7 +107,7 @@ RSpec.configure do |config|
     in_app_root
     @command_executions = []
 
-    example.run
+    Bundler.ui.silence { example.run }
 
     all_output = @command_executions.map(&:to_s_verbose).join("\n\n")
     if example.exception && !all_output.empty?
diff --git a/spec/support/filters.rb b/spec/support/filters.rb
index 4ce6648cdc..2c4c032ea3 100644
--- a/spec/support/filters.rb
+++ b/spec/support/filters.rb
@@ -39,7 +39,6 @@ RSpec.configure do |config|
   config.filter_run_excluding :git => RequirementChecker.against(git_version)
   config.filter_run_excluding :bundler => RequirementChecker.against(Bundler::VERSION.split(".")[0])
   config.filter_run_excluding :ruby_repo => !ENV["GEM_COMMAND"].nil?
-  config.filter_run_excluding :no_color_tty => Gem.win_platform? || !ENV["GITHUB_ACTION"].nil?
 
   config.filter_run_when_matching :focus unless ENV["CI"]
 end
diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb
index ffcde39df7..76a1f77b72 100644
--- a/spec/support/helpers.rb
+++ b/spec/support/helpers.rb
@@ -19,8 +19,6 @@ module Spec
       FileUtils.mkdir_p(home)
       FileUtils.mkdir_p(tmpdir)
       Bundler.reset!
-      Bundler.ui = nil
-      Bundler.ui # force it to initialize
     end
 
     def self.bang(method)
@@ -80,7 +78,7 @@ module Spec
     def run(cmd, *args)
       opts = args.last.is_a?(Hash) ? args.pop : {}
       groups = args.map(&:inspect).join(", ")
-      setup = "require '#{lib}/bundler' ; Bundler.setup(#{groups})\n"
+      setup = "require '#{lib}/bundler' ; Bundler.ui.silence { Bundler.setup(#{groups}) }\n"
       ruby(setup + cmd, opts)
     end
     bang :run