Skip to content

Commit

Permalink
Merge pull request #7100 from voxik/less-exceptional-user-install
Browse files Browse the repository at this point in the history
Less exceptional user install
  • Loading branch information
duckinator committed Oct 25, 2023
2 parents 9bdad37 + 313b1c5 commit 2314d28
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 42 deletions.
8 changes: 0 additions & 8 deletions lib/rubygems/commands/install_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,6 @@ def usage # :nodoc:
"#{program_name} [options] GEMNAME [GEMNAME ...] -- --build-flags"
end

def check_install_dir # :nodoc:
if options[:install_dir] && options[:user_install]
alert_error "Use --install-dir or --user-install but not both"
terminate_interaction 1
end
end

def check_version # :nodoc:
if options[:version] != Gem::Requirement.default &&
get_all_gem_names.size > 1
Expand All @@ -162,7 +155,6 @@ def execute

ENV.delete "GEM_PATH" if options[:install_dir].nil?

check_install_dir
check_version

load_hooks
Expand Down
23 changes: 11 additions & 12 deletions lib/rubygems/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,6 @@ def initialize(package, options={})
@package.prog_mode = options[:prog_mode]
@package.data_mode = options[:data_mode]

if options[:user_install]
@gem_home = Gem.user_dir
@bin_dir = Gem.bindir gem_home unless options[:bin_dir]
@plugins_dir = Gem.plugindir(gem_home)
end

if @gem_home == Gem.user_dir
# If we get here, then one of the following likely happened:
# - `--user-install` was specified
Expand Down Expand Up @@ -673,22 +667,27 @@ def process_options # :nodoc:
@env_shebang = options[:env_shebang]
@force = options[:force]
@install_dir = options[:install_dir]
@gem_home = options[:install_dir] || Gem.dir
@plugins_dir = Gem.plugindir(@gem_home)
@ignore_dependencies = options[:ignore_dependencies]
@format_executable = options[:format_executable]
@wrappers = options[:wrappers]
@only_install_dir = options[:only_install_dir]

# 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
# (or use) a new bin dir under the gem_home.
@bin_dir = options[:bin_dir] || Gem.bindir(gem_home)
@bin_dir = options[:bin_dir]
@development = options[:development]
@build_root = options[:build_root]

@build_args = options[:build_args]

@gem_home = @install_dir
@gem_home ||= options[:user_install] ? Gem.user_dir : Gem.dir

# 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
# (or use) a new bin dir under the gem_home.
@bin_dir ||= Gem.bindir(@gem_home)

@plugins_dir = Gem.plugindir(@gem_home)

unless @build_root.nil?
@bin_dir = File.join(@build_root, @bin_dir.gsub(/^[a-zA-Z]:/, ""))
@gem_home = File.join(@build_root, @gem_home.gsub(/^[a-zA-Z]:/, ""))
Expand Down
10 changes: 4 additions & 6 deletions test/rubygems/installer_test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def util_make_exec(spec = @spec, shebang = "#!/usr/bin/ruby", bindir = "bin")

def setup_base_installer(force = true)
@gem = setup_base_gem
util_installer @spec, @gemhome, false, force
util_installer @spec, @gemhome, force
end

##
Expand Down Expand Up @@ -163,7 +163,7 @@ def setup_base_user_installer

@user_gem = @user_spec.cache_file

util_installer @user_spec, Gem.user_dir, :user
Gem::Installer.at @user_gem, :user_install => true
end

##
Expand Down Expand Up @@ -219,13 +219,11 @@ def util_setup_gem(ui = @ui, force = true)
end

##
# Creates an installer for +spec+ that will install into +gem_home+. If
# +user+ is true a user-install will be performed.
# Creates an installer for +spec+ that will install into +gem_home+.

def util_installer(spec, gem_home, user=false, force=true)
def util_installer(spec, gem_home, force=true)
Gem::Installer.at(spec.cache_file,
:install_dir => gem_home,
:user_install => user,
:force => force)
end

Expand Down
15 changes: 0 additions & 15 deletions test/rubygems/test_gem_commands_install_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -436,21 +436,6 @@ def test_execute_nonexistent_with_dashes
assert_equal expected, output
end

def test_execute_conflicting_install_options
@cmd.options[:user_install] = true
@cmd.options[:install_dir] = "whatever"

use_ui @ui do
assert_raise Gem::MockGemUi::TermError do
@cmd.execute
end
end

expected = "ERROR: Use --install-dir or --user-install but not both\n"

assert_equal expected, @ui.error
end

def test_execute_prerelease_skipped_when_no_flag_set
spec_fetcher do |fetcher|
fetcher.gem "a", 1
Expand Down
15 changes: 14 additions & 1 deletion test/rubygems/test_gem_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ def test_generate_plugins_with_user_install
File.chmod(0o555, Gem.plugindir)
system_path = File.join(Gem.plugindir, "a_plugin.rb")
user_path = File.join(Gem.plugindir(Gem.user_dir), "a_plugin.rb")
installer = util_installer spec, Gem.dir, :user
installer = Gem::Installer.at spec.cache_file, :user_install => true, :force => true

assert_equal spec, installer.install

Expand Down Expand Up @@ -995,6 +995,19 @@ def test_initialize_user_install_bin_dir
assert_equal @tempdir, installer.bin_dir
end

def test_install_dir_takes_precedence_to_user_install
gemhome2 = "#{@gemhome}2"

@gem = setup_base_gem

installer =
Gem::Installer.at @gem, :install_dir => gemhome2, :user_install => true
installer.install

assert_path_exist File.join(gemhome2, "gems", @spec.full_name)
assert_path_not_exist File.join(Gem.user_dir, "gems", @spec.full_name)
end

def test_install
installer = util_setup_installer

Expand Down

0 comments on commit 2314d28

Please sign in to comment.