[rubygems/rubygems] Better approach to falling back to user installat…

…ion when GEM_HOME not writable rubygems/rubygems@f67bced16b
ruby · Dec 7, 2023 · 33bd956 · 33bd956
1 parent 0f3f907
commit 33bd956
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 128 deletions.
diff --git a/lib/rubygems/command.rb b/lib/rubygems/command.rb
@@ -22,15 +22,6 @@ class Gem::Command
 
   Gem::OptionParser.accept Symbol, &:to_sym
 
-  ##
-  # Names of commands that should print "Defaulting to user installation"
-  # warning.
-
-  COMMANDS_WITH_AUTO_INSTALL_DIR_WARNING = [
-    "install",
-    "update",
-  ].freeze
-
   ##
   # The name of the command.
 
@@ -332,11 +323,6 @@ def invoke_with_build_args(args, build_args)
     elsif @when_invoked
       @when_invoked.call options
     else
-      if COMMANDS_WITH_AUTO_INSTALL_DIR_WARNING.include?(@command) && \
-         Gem.paths.auto_user_install && !options[:install_dir] && !options[:user_install]
-        self.ui.say "Defaulting to user installation because default installation directory (#{Gem.default_dir}) is not writable."
-      end
-
       execute
     end
   ensure

diff --git a/lib/rubygems/installer.rb b/lib/rubygems/installer.rb
@@ -676,7 +676,14 @@ def process_options # :nodoc:
     @build_args = options[:build_args]
 
     @gem_home = @install_dir
-    @gem_home ||= options[:user_install] ? Gem.user_dir : Gem.dir
+    @gem_home ||= if options[:user_install]
+      Gem.user_dir
+    elsif !ENV.key?("GEM_HOME") && (File.exist?(Gem.dir) && !File.writable?(Gem.dir))
+      say "Defaulting to user installation because default installation directory (#{Gem.dir}) is not writable."
+      Gem.user_dir
+    else
+      Gem.dir
+    end
 
     # If the user has asked for the gem to be installed in a directory that is
     # the system gem directory, then use the system bin directory, else create

diff --git a/lib/rubygems/path_support.rb b/lib/rubygems/path_support.rb
@@ -18,32 +18,13 @@ class Gem::PathSupport
   # Directory with spec cache
   attr_reader :spec_cache_dir # :nodoc:
 
-  ##
-  # Whether `Gem.paths.home` defaulted to a user install or not.
-  attr_reader :auto_user_install
-
   ##
   #
   # Constructor. Takes a single argument which is to be treated like a
   # hashtable, or defaults to ENV, the system environment.
   #
   def initialize(env)
-    # Current implementation of @home, which is exposed as `Gem.paths.home`:
-    # 1. If `env["GEM_HOME"]` is defined in the environment: `env["GEM_HOME"]`.
-    # 2. If `Gem.default_dir` is writable: `Gem.default_dir`.
-    # 3. Otherwise: `Gem.user_dir`.
-
-    if env.key?("GEM_HOME")
-      @home = normalize_home_dir(env["GEM_HOME"])
-    elsif File.writable?(Gem.default_dir)
-      @home = normalize_home_dir(Gem.default_dir)
-    else
-      # If `GEM_HOME` is not set AND we can't use `Gem.default_dir`,
-      # default to a user installation and set `@auto_user_install`.
-      @auto_user_install = true
-      @home = normalize_home_dir(Gem.user_dir)
-    end
-
+    @home = normalize_home_dir(env["GEM_HOME"] || Gem.default_dir)
     @path = split_gem_path env["GEM_PATH"], @home
 
     @spec_cache_dir = env["GEM_SPEC_CACHE"] || Gem.default_spec_cache_dir

diff --git a/test/rubygems/test_gem_command.rb b/test/rubygems/test_gem_command.rb
@@ -399,66 +399,4 @@ def test_show_lookup_failure_suggestions_remote
 
     assert_equal expected, @ui.error
   end
-
-  def test_show_defaulting_to_user_install_when_appropriate
-    omit "this test doesn't work with ruby-core setup" if ruby_repo?
-
-    Gem.stub(:default_dir, "/this-directory-does-not-exist") do
-      # Replace `Gem.paths` with a new instance, so `Gem.paths.auto_user_install`
-      # is accurate.
-      Gem.stub(:paths, Gem::PathSupport.new(ENV)) do
-        output_regex = "Defaulting to user installation"
-
-        test_command = Class.new(Gem::Command) do
-          def initialize
-            # "gem install" should ALWAYS print the warning.
-            super("install", "Gem::Command instance for testing")
-          end
-
-          def execute
-            true
-          end
-        end
-
-        cmd = test_command.new
-
-        use_ui @ui do
-          cmd.invoke
-          assert_match output_regex, @ui.output,
-            "Gem.default_dir = #{Gem.default_dir.inspect}\n" \
-            "Gem.paths.auto_user_install = #{Gem.paths.auto_user_install.inspect}"
-        end
-      end
-    end
-  end
-
-  def test_dont_show_defaulting_to_user_install_when_appropriate
-    Gem.stub(:default_dir, "/this-directory-does-not-exist") do
-      # Replace `Gem.paths` with a new instance, so `Gem.paths.auto_user_install`
-      # is accurate.
-      Gem.stub(:paths, Gem::PathSupport.new(ENV)) do
-        output_regex = /^Defaulting to user installation/
-
-        test_command = Class.new(Gem::Command) do
-          def initialize
-            # "gem blargh" should NEVER print the warning.
-            super("blargh", "Gem::Command instance for testing")
-          end
-
-          def execute
-            true
-          end
-        end
-
-        cmd = test_command.new
-
-        use_ui @ui do
-          cmd.invoke
-          assert_no_match output_regex, @ui.output,
-            "Gem.default_dir = #{Gem.default_dir.inspect}\n" \
-            "Gem.paths.auto_user_install = #{Gem.paths.auto_user_install.inspect}"
-        end
-      end
-    end
-  end
 end
diff --git a/test/rubygems/test_gem_install_update_options.rb b/test/rubygems/test_gem_install_update_options.rb
@@ -156,37 +156,6 @@ def test_user_install_disabled_read_only
     FileUtils.chmod 0o755, @gemhome
   end
 
-  def test_auto_install_dir_unless_gem_home_writable
-    if Process.uid.zero?
-      pend("test_auto_install_dir_unless_gem_home_writable test skipped in root privilege")
-      return
-    end
-
-    orig_gem_home = ENV["GEM_HOME"]
-    ENV.delete("GEM_HOME")
-
-    @spec = quick_gem "a" do |spec|
-      util_make_exec spec
-    end
-
-    util_build_gem @spec
-    @gem = @spec.cache_file
-
-    @cmd.handle_options %w[]
-
-    assert_not_equal Gem.paths.home, Gem.user_dir
-
-    FileUtils.chmod 0o755, @userhome
-    FileUtils.chmod 0o000, @gemhome
-
-    Gem.use_paths nil, @userhome
-
-    assert_equal Gem.paths.home, Gem.user_dir
-  ensure
-    FileUtils.chmod 0o755, @gemhome
-    ENV["GEM_HOME"] = orig_gem_home if orig_gem_home
-  end
-
   def test_vendor
     vendordir(File.join(@tempdir, "vendor")) do
       @cmd.handle_options %w[--vendor]

diff --git a/test/rubygems/test_gem_installer.rb b/test/rubygems/test_gem_installer.rb
@@ -1955,6 +1955,26 @@ def test_process_options_build_root
     assert_equal "  Plugins dir: #{plugins_dir}", errors.shift
   end
 
+  def test_process_options_fallback_to_user_install_when_gem_home_not_writable
+    if Process.uid.zero?
+      pend("skipped in root privilege")
+      return
+    end
+
+    orig_gem_home = ENV.delete("GEM_HOME")
+
+    @gem = setup_base_gem
+
+    FileUtils.chmod 0o000, @gemhome
+
+    use_ui(@ui) { Gem::Installer.at @gem }
+
+    assert_equal "Defaulting to user installation because default installation directory (#{@gemhome}) is not writable.", @ui.output.strip
+  ensure
+    FileUtils.chmod 0o755, @gemhome
+    ENV["GEM_HOME"] = orig_gem_home
+  end
+
   def test_shebang_arguments
     load_relative "no" do
       installer = setup_base_installer