Skip to content

Commit 1c282d9

Browse files
Fix gem install sometimes compiling the wrong source files
If a previous copy of a gem is already installed, RubyGems will not reinstall the gem but only recompile its extensions. This seems like a good idea, but only if the gem is being installed from the registry. If we are installing a locally built package, then the package should be completely reinstalled and extensions compiled from the sources in the locally built package, not from the sources in the previous installation.
1 parent 9a85907 commit 1c282d9

2 files changed

Lines changed: 55 additions & 6 deletions

File tree

lib/rubygems/request_set.rb

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,10 @@ def install(options, &block) # :yields: request, installer
181181

182182
# Install requested gems after they have been downloaded
183183
sorted_requests.each do |req|
184-
if req.installed?
184+
if req.installed? && @always_install.none? {|spec| spec == req.spec.spec }
185185
req.spec.spec.build_extensions
186-
187-
if @always_install.none? {|spec| spec == req.spec.spec }
188-
yield req, nil if block_given?
189-
next
190-
end
186+
yield req, nil if block_given?
187+
next
191188
end
192189

193190
spec =

test/rubygems/test_gem_dependency_installer.rb

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,58 @@ def test_install_local_subdir
519519
assert_equal %w[a-1], inst.installed_gems.map(&:full_name)
520520
end
521521

522+
def test_install_local_with_extensions_already_installed
523+
pend "needs investigation" if Gem.java_platform?
524+
pend "ruby.h is not provided by ruby repo" if ruby_repo?
525+
526+
@spec = quick_gem "a" do |s|
527+
s.extensions << "extconf.rb"
528+
s.files += %w[extconf.rb a.c]
529+
end
530+
531+
write_dummy_extconf "a"
532+
533+
c_source_path = File.join(@tempdir, "a.c")
534+
535+
write_file c_source_path do |io|
536+
io.write <<-C
537+
#include <ruby.h>
538+
void Init_a() { }
539+
C
540+
end
541+
542+
package_path = Gem::Package.build @spec
543+
installer = Gem::Installer.at(package_path)
544+
545+
# Make sure the gem is installed and backup the correct package
546+
547+
installer.install
548+
549+
package_bkp_path = "#{package_path}.bkp"
550+
FileUtils.cp package_path, package_bkp_path
551+
552+
# Break the extension, rebuild it, and try to install it
553+
554+
write_file c_source_path do |io|
555+
io.write "typo"
556+
end
557+
558+
Gem::Package.build @spec
559+
560+
assert_raise Gem::Ext::BuildError do
561+
installer.install
562+
end
563+
564+
# Make sure installing the good package again still works
565+
566+
FileUtils.cp "#{package_path}.bkp", package_path
567+
568+
Dir.chdir @tempdir do
569+
inst = Gem::DependencyInstaller.new domain: :local
570+
inst.install package_path
571+
end
572+
end
573+
522574
def test_install_minimal_deps
523575
util_setup_gems
524576

0 commit comments

Comments
 (0)