Skip to content

Commit

Permalink
[rubygems/rubygems] Warning about PATH in --user-install mode is on…
Browse files Browse the repository at this point in the history
…ly necessary for gems with executables

rubygems/rubygems@2fe0f452a2
  • Loading branch information
deivid-rodriguez authored and matzbot committed Sep 27, 2024
1 parent b873787 commit d132417
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 36 deletions.
37 changes: 14 additions & 23 deletions lib/rubygems/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ class Gem::Installer

attr_reader :package

@path_warning = false

class << self
#
# Changes in rubygems to lazily loading `rubygems/command` (in order to
Expand All @@ -86,11 +84,6 @@ def inherited(klass)
super(klass)
end

##
# True if we've warned about PATH not including Gem.bindir

attr_accessor :path_warning

##
# Overrides the executable format.
#
Expand Down Expand Up @@ -188,15 +181,6 @@ def initialize(package, options={})
@package.dir_mode = options[:dir_mode]
@package.prog_mode = options[:prog_mode]
@package.data_mode = options[:data_mode]

if @gem_home == Gem.user_dir
# If we get here, then one of the following likely happened:
# - `--user-install` was specified
# - `Gem::PathSupport#home` fell back to `Gem.user_dir`
# - GEM_HOME was manually set to `Gem.user_dir`

check_that_user_bin_dir_is_in_path
end
end

##
Expand Down Expand Up @@ -488,11 +472,21 @@ def generate_windows_script(filename, bindir)
end

def generate_bin # :nodoc:
return if spec.executables.nil? || spec.executables.empty?
executables = spec.executables
return if executables.nil? || executables.empty?

if @gem_home == Gem.user_dir
# If we get here, then one of the following likely happened:
# - `--user-install` was specified
# - `Gem::PathSupport#home` fell back to `Gem.user_dir`
# - GEM_HOME was manually set to `Gem.user_dir`

check_that_user_bin_dir_is_in_path(executables)
end

ensure_writable_dir @bin_dir

spec.executables.each do |filename|
executables.each do |filename|
bin_path = File.join gem_dir, spec.bindir, filename
next unless File.exist? bin_path

Expand Down Expand Up @@ -694,9 +688,7 @@ def process_options # :nodoc:
end
end

def check_that_user_bin_dir_is_in_path # :nodoc:
return if self.class.path_warning

def check_that_user_bin_dir_is_in_path(executables) # :nodoc:
user_bin_dir = @bin_dir || Gem.bindir(gem_home)
user_bin_dir = user_bin_dir.tr(File::ALT_SEPARATOR, File::SEPARATOR) if File::ALT_SEPARATOR

Expand All @@ -712,8 +704,7 @@ def check_that_user_bin_dir_is_in_path # :nodoc:

unless path.include? user_bin_dir
unless !Gem.win_platform? && (path.include? user_bin_dir.sub(ENV["HOME"], "~"))
alert_warning "You don't have #{user_bin_dir} in your PATH,\n\t gem executables will not run."
self.class.path_warning = true
alert_warning "You don't have #{user_bin_dir} in your PATH,\n\t gem executables (#{executables.join(", ")}) will not run."
end
end
end
Expand Down
6 changes: 0 additions & 6 deletions test/rubygems/installer_test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,6 @@ class Gem::Installer
# A test case for Gem::Installer.

class Gem::InstallerTestCase < Gem::TestCase
def setup
super

Gem::Installer.path_warning = false
end

##
# The path where installed executables live

Expand Down
13 changes: 6 additions & 7 deletions test/rubygems/test_gem_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def test_check_that_user_bin_dir_is_in_path
ENV["PATH"] = [ENV["PATH"], bin_dir].join(File::PATH_SEPARATOR)

use_ui @ui do
installer.check_that_user_bin_dir_is_in_path
installer.check_that_user_bin_dir_is_in_path(["executable"])
end

assert_empty @ui.error
Expand All @@ -218,7 +218,7 @@ def test_check_that_user_bin_dir_is_in_path
ENV["PATH"] = [orig_path, bin_dir.tr(File::SEPARATOR, File::ALT_SEPARATOR)].join(File::PATH_SEPARATOR)

use_ui @ui do
installer.check_that_user_bin_dir_is_in_path
installer.check_that_user_bin_dir_is_in_path(["executable"])
end

assert_empty @ui.error
Expand All @@ -236,7 +236,7 @@ def test_check_that_user_bin_dir_is_in_path_tilde
installer.bin_dir.replace File.join @userhome, "bin"

use_ui @ui do
installer.check_that_user_bin_dir_is_in_path
installer.check_that_user_bin_dir_is_in_path(["executable"])
end

assert_empty @ui.error
Expand All @@ -248,7 +248,7 @@ def test_check_that_user_bin_dir_is_in_path_not_in_path
installer = setup_base_installer

use_ui @ui do
installer.check_that_user_bin_dir_is_in_path
installer.check_that_user_bin_dir_is_in_path(["executable"])
end

expected = installer.bin_dir
Expand All @@ -258,6 +258,7 @@ def test_check_that_user_bin_dir_is_in_path_not_in_path
end

assert_match expected, @ui.error
assert_match "(executable)", @ui.error
end

def test_ensure_dependency
Expand Down Expand Up @@ -358,10 +359,8 @@ def test_generate_bin_bindir_with_user_install_warning

inst = Gem::Installer.at "", options

Gem::Installer.path_warning = false

use_ui @ui do
inst.check_that_user_bin_dir_is_in_path
inst.check_that_user_bin_dir_is_in_path(["executable"])
end

assert_equal "", @ui.error
Expand Down

0 comments on commit d132417

Please sign in to comment.