diff --git a/lib/bundler/rubygems_ext.rb b/lib/bundler/rubygems_ext.rb index 4ad2bdf46f04..6025fad97294 100644 --- a/lib/bundler/rubygems_ext.rb +++ b/lib/bundler/rubygems_ext.rb @@ -235,6 +235,20 @@ def extension_dir end end + alias_method :rg_build_info_dir, :build_info_dir + def build_info_dir + # A git checkout's build logs belong with that checkout's extension build. + # base_dir points at the directory holding every checkout, so logs keyed by + # full_name would collide between revisions of the same gem and would sit + # outside anything `bundle clean` prunes. extension_dir is unique per + # revision and goes away with the checkout. + if source.respond_to?(:extension_dir_name) + extension_dir + else + rg_build_info_dir + end + end + # Can be removed once RubyGems 3.5.21 support is dropped remove_method :gem_dir if method_defined?(:gem_dir, false) diff --git a/lib/rubygems/commands/install_command.rb b/lib/rubygems/commands/install_command.rb index 2ebbc40a0308..288da7c31fee 100644 --- a/lib/rubygems/commands/install_command.rb +++ b/lib/rubygems/commands/install_command.rb @@ -93,7 +93,7 @@ def description # :nodoc: [build fails] Gem files will remain installed in \\ /path/to/gems/some_extension_gem-1.0 for inspection. - Results logged to /path/to/gems/some_extension_gem-1.0/gem_make.out + Results logged to /path/to/build_info/some_extension_gem-1.0.gem_make.out $ gem install some_extension_gem -- --with-extension-lib=/path/to/lib [build succeeds] $ gem list some_extension_gem @@ -110,7 +110,7 @@ def description # :nodoc: [build fails] Gem files will remain installed in \\ /path/to/gems/some_extension_gem-1.0 for inspection. - Results logged to /path/to/gems/some_extension_gem-1.0/gem_make.out + Results logged to /path/to/build_info/some_extension_gem-1.0.gem_make.out $ [cd /path/to/gems/some_extension_gem-1.0] $ [edit files or what-have-you and run make] $ gem spec ../../cache/some_extension_gem-1.0.gem --ruby > \\ diff --git a/lib/rubygems/doctor.rb b/lib/rubygems/doctor.rb index 4f26260d836a..bb20e9314961 100644 --- a/lib/rubygems/doctor.rb +++ b/lib/rubygems/doctor.rb @@ -21,7 +21,7 @@ class Gem::Doctor REPOSITORY_EXTENSION_MAP = [ # :nodoc: ["specifications", ".gemspec"], - ["build_info", ".info"], + ["build_info", ".info", ".mkmf.log", ".gem_make.out"], ["cache", ".gem"], ["doc", ""], ["extensions", ""], @@ -92,15 +92,15 @@ def doctor # Cleans up children of this gem repository def doctor_children # :nodoc: - REPOSITORY_EXTENSION_MAP.each do |sub_directory, extension| - doctor_child sub_directory, extension + REPOSITORY_EXTENSION_MAP.each do |sub_directory, *extensions| + doctor_child sub_directory, *extensions end end ## - # Removes files in +sub_directory+ with +extension+ + # Removes files in +sub_directory+ with any of +extensions+ - def doctor_child(sub_directory, extension) # :nodoc: + def doctor_child(sub_directory, *extensions) # :nodoc: directory = File.join(@gem_repository, sub_directory) Dir.entries(directory).sort.each do |ent| @@ -109,7 +109,7 @@ def doctor_child(sub_directory, extension) # :nodoc: child = File.join(directory, ent) next unless File.exist?(child) - basename = File.basename(child, extension) + basename = strip_extension File.basename(child), extensions next if installed_specs.include? basename next if /^rubygems-\d/.match?(basename) next if sub_directory == "specifications" && basename == "default" @@ -129,4 +129,15 @@ def doctor_child(sub_directory, extension) # :nodoc: rescue Errno::ENOENT # ignore end + + ## + # Removes the first of +extensions+ that +name+ ends with + + def strip_extension(name, extensions) # :nodoc: + extension = extensions.find do |ext| + !ext.empty? && name.end_with?(ext) + end + + extension ? name.delete_suffix(extension) : name + end end diff --git a/lib/rubygems/ext/builder.rb b/lib/rubygems/ext/builder.rb index f1fce48823a8..b7e80d6b06bb 100644 --- a/lib/rubygems/ext/builder.rb +++ b/lib/rubygems/ext/builder.rb @@ -220,9 +220,11 @@ def build_error(output, backtrace = nil) # :nodoc: #{output} Gem files will remain installed in #{@gem_dir} for inspection. -Results logged to #{gem_make_out} EOF + # Losing the log must not cost the user the build error itself. + message += "Results logged to #{gem_make_out}\n" if gem_make_out + raise Gem::Ext::BuildError, message, backtrace end @@ -243,13 +245,72 @@ def build_extension(extension, dest_path) # :nodoc: verbose { results.join("\n") } - write_gem_make_out results.join "\n" + # Build logs are noisy, non-reproducible artifacts that are not meant to + # be installed. Drop the ones this build left behind, plus any written + # into the extension directory by a RubyGems old enough to put them there. + FileUtils.rm_f mkmf_log_candidates(extension_dir, dest_path) + FileUtils.rm_f File.join(dest_path, "gem_make.out") + FileUtils.rm_f [build_log_path("mkmf.log"), build_log_path("gem_make.out")] + rescue Gem::Ext::Builder::NoMakefileError => e + # extconf ran fine but produced no Makefile, so the extension was skipped + # rather than built and installing carries on. Keep the log that says why + # it was skipped, out of the installation tree but still reachable. + results << e.message + results << "Skipping make for #{extension} as no Makefile was found." + + verbose { results.join("\n") } + + preserve_mkmf_log extension_dir, dest_path + write_gem_make_out results.join("\n") rescue StandardError => e results << e.message + + mkmf_log_dest = preserve_mkmf_log(extension_dir, dest_path) + if mkmf_log_dest + results << "To see why this extension failed to compile, please check the mkmf.log which can be found here:" + results << " #{mkmf_log_dest}" + end + build_error(results.join("\n"), $@) end end + ## + # Where a build log of +kind+ for this gem lives in the build_info directory. + + def build_log_path(kind) # :nodoc: + File.join @spec.build_info_dir, "#{@spec.full_name}.#{kind}" + end + + ## + # Moves the mkmf.log this build left behind into the build_info directory and + # returns its new path, or nil when there is none or it cannot be kept. + # Keeping a log must never replace the build error the caller is reporting, + # so a filesystem failure here is swallowed. + + def preserve_mkmf_log(extension_dir, dest_path) # :nodoc: + mkmf_log = mkmf_log_candidates(extension_dir, dest_path).find {|log| File.exist?(log) } + return unless mkmf_log + + destination = build_log_path "mkmf.log" + + FileUtils.mkdir_p @spec.build_info_dir + FileUtils.mv mkmf_log, destination + + destination + rescue SystemCallError + nil + end + + ## + # Places a completed build may have left an mkmf.log, most specific first. + # Gem::Ext::ExtConfBuilder parks it in +dest_path+ so that the "clean" target + # cannot delete it; the other builders leave it where extconf ran. + + def mkmf_log_candidates(extension_dir, dest_path) # :nodoc: + [File.join(dest_path, "mkmf.log"), File.join(extension_dir, "mkmf.log")] + end + ## # Builds extensions. Valid types of extensions are extconf.rb files, # configure scripts and rakefiles or mkrf_conf files. @@ -277,17 +338,21 @@ def build_extensions end ## - # Writes +output+ to gem_make.out in the extension install directory. + # Writes +output+ to gem_make.out in the build_info directory and returns its + # path, or nil when it cannot be written. Only called when the extension was + # not built, to keep build logs out of the installation tree. def write_gem_make_out(output) # :nodoc: - destination = File.join @spec.extension_dir, "gem_make.out" + destination = build_log_path "gem_make.out" - FileUtils.mkdir_p @spec.extension_dir + FileUtils.mkdir_p @spec.build_info_dir File.open destination, "wb" do |io| io.puts output end destination + rescue SystemCallError + nil end end diff --git a/lib/rubygems/ext/ext_conf_builder.rb b/lib/rubygems/ext/ext_conf_builder.rb index 822454355d10..9cf7eafc5dc3 100644 --- a/lib/rubygems/ext/ext_conf_builder.rb +++ b/lib/rubygems/ext/ext_conf_builder.rb @@ -27,17 +27,14 @@ def self.build(extension, dest_path, results, args = [], lib_dir = nil, extensio cmd << "--target-rbconfig=#{target_rbconfig.path}" if target_rbconfig.path cmd.push(*args) - run(cmd, results, class_name, extension_dir) do |s, r| - mkmf_log = File.join(extension_dir, "mkmf.log") - if File.exist? mkmf_log - unless s.success? - r << "To see why this extension failed to compile, please check" \ - " the mkmf.log which can be found here:\n" - r << " " + File.join(dest_path, "mkmf.log") + "\n" - end - FileUtils.mv mkmf_log, dest_path - end - end + run(cmd, results, class_name, extension_dir) + + # "clean" is the first make target, and mkmf puts mkmf.log in CLEANFILES, + # so park the log next to the built extension before make can delete it. + # Whether it is then dropped or kept for inspection is decided by + # Gem::Ext::Builder#build_extension. + mkmf_log = File.join(extension_dir, "mkmf.log") + FileUtils.mv mkmf_log, dest_path if File.exist?(mkmf_log) ENV["DESTDIR"] = nil @@ -66,10 +63,6 @@ def self.build(extension, dest_path, results, args = [], lib_dir = nil, extensio end results - rescue Gem::Ext::Builder::NoMakefileError => error - results << error.message - results << "Skipping make for #{extension} as no Makefile was found." - # We are good, do not re-raise the error. ensure FileUtils.rm_rf tmp_dest if tmp_dest end diff --git a/lib/rubygems/uninstaller.rb b/lib/rubygems/uninstaller.rb index 28bf33ea7106..f87ee2f49841 100644 --- a/lib/rubygems/uninstaller.rb +++ b/lib/rubygems/uninstaller.rb @@ -272,6 +272,8 @@ def remove(spec) safe_delete { rm_r full_gem_path, exclusions: exclusions } safe_delete { FileUtils.rm_r spec.extension_dir } + safe_delete { FileUtils.rm_f File.join(spec.build_info_dir, "#{spec.full_name}.mkmf.log") } + safe_delete { FileUtils.rm_f File.join(spec.build_info_dir, "#{spec.full_name}.gem_make.out") } old_platform_name = spec.original_name diff --git a/spec/bundler/installer/parallel_installer_spec.rb b/spec/bundler/installer/parallel_installer_spec.rb index 20d39e885be3..29c661134b6f 100644 --- a/spec/bundler/installer/parallel_installer_spec.rb +++ b/spec/bundler/installer/parallel_installer_spec.rb @@ -152,34 +152,37 @@ let(:gem_two) { definition.specs.find {|spec| spec.name == "two" } } it "takes all available slots" do - redefine_build_jobs do + acquired = track_build_jobs(rendezvous: true) do Bundler::ParallelInstaller.call(installer, definition.specs, 5, false, true) end - # Take 3 slots out of the 5 available. - expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3") + # Take 3 slots (capped per gem) out of the 5 available. + expect(acquired["one"]).to eq(3) # Take the remaining 2 slots. - expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to include("make -j2") + expect(acquired["two"]).to eq(2) end it "fallback to non parallel when no slots are available" do - redefine_build_jobs do + acquired = track_build_jobs(rendezvous: true) do Bundler::ParallelInstaller.call(installer, definition.specs, 3, false, true) end # Take 3 slots out of the 3 available. - expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3") + expect(acquired["one"]).to eq(3) # Fallback to one slot (non parallel). - expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to_not include("make -j") + expect(acquired["two"]).to eq(1) end it "uses one jobs when installing serially" do + acquired = nil Bundler.settings.temporary(jobs: 1) do - Bundler::ParallelInstaller.call(installer, definition.specs, 1, false, true) + acquired = track_build_jobs do + Bundler::ParallelInstaller.call(installer, definition.specs, 1, false, true) + end end - expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to_not include("make -j") - expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to_not include("make -j") + expect(acquired["one"]).to eq(1) + expect(acquired["two"]).to eq(1) end it "release the job slots" do @@ -191,39 +194,52 @@ end end - Bundler::ParallelInstaller.call(installer, definition.specs, 3, false, true) + acquired = track_build_jobs do + Bundler::ParallelInstaller.call(installer, definition.specs, 3, false, true) + end # Take 3 slots out of the 3 available. - expect(File.read(File.join(gem_one.extension_dir, "gem_make.out"))).to include("make -j3") - # Take 3 slots that were released. - expect(File.read(File.join(gem_two.extension_dir, "gem_make.out"))).to include("make -j3") + expect(acquired["one"]).to eq(3) + # Take 3 slots that were released by `one`. + expect(acquired["two"]).to eq(3) end - def redefine_build_jobs + # Records how many jobserver slots each gem's build acquired. RubyGems turns + # that count directly into `make -jN`, so asserting on it verifies slot + # allocation and release without reading a build log, which a successful + # build no longer writes. With +rendezvous+, "one" grabs its slots first and + # holds them until "two" has grabbed the rest, making the split deterministic. + def track_build_jobs(rendezvous: false) + acquired = {} old_method = Bundler::RubyGemsGemInstaller.instance_method(:build_jobs) Bundler::RubyGemsGemInstaller.remove_method(:build_jobs) - # Rendezvous so that "one" grabs its slots first and keeps holding them - # until "two" has grabbed the rest. Blocking on a queue avoids the - # busy-wait and makes the ordering deterministic. one_acquired = Thread::Queue.new two_acquired = Thread::Queue.new Bundler::RubyGemsGemInstaller.define_method(:build_jobs) do - if spec.name == "one" - value = old_method.bind(self).call - one_acquired << true - two_acquired.pop - elsif spec.name == "two" - one_acquired.pop - value = old_method.bind(self).call - two_acquired << true - end + value = + if rendezvous && spec.name == "one" + v = old_method.bind(self).call + one_acquired << true + two_acquired.pop + v + elsif rendezvous && spec.name == "two" + one_acquired.pop + v = old_method.bind(self).call + two_acquired << true + v + else + old_method.bind(self).call + end + acquired[spec.name] = value value end yield + + acquired ensure Bundler::RubyGemsGemInstaller.remove_method(:build_jobs) Bundler::RubyGemsGemInstaller.define_method(:build_jobs, old_method) diff --git a/spec/commands/install_spec.rb b/spec/commands/install_spec.rb index a0b56a3d2970..d4b88902e21e 100644 --- a/spec/commands/install_spec.rb +++ b/spec/commands/install_spec.rb @@ -1369,10 +1369,18 @@ def run s.extensions = extension s.write(extension, extconf_code) + # A successful build no longer leaves gem_make.out behind. Force the + # build to fail at the make stage so the make command line, including + # the jobserver `-j`, is recorded in build_info for these assertions. + s.write("ext/mypsych/mypsych.c", "#error forced build failure for test") end end end + def gem_make_out + File.read(File.join(@gemspec.build_info_dir, "#{@gemspec.full_name}.gem_make.out")) + end + after do if @old_makeflags ENV["MAKEFLAGS"] = @old_makeflags @@ -1381,42 +1389,36 @@ def run end end - it "doesn't pass down -j to make when MAKEFLAGS is set" do + it "doesn't pass down -j to make when MAKEFLAGS is set", rubygems: ">= 4.1.0.dev" do ENV["MAKEFLAGS"] = "-j1" - install_gemfile(<<~G, env: { "BUNDLE_JOBS" => "8" }) + install_gemfile(<<~G, env: { "BUNDLE_JOBS" => "8" }, raise_on_error: false) source "https://gem.repo4" gem "mypsych" G - gem_make_out = File.read(File.join(@gemspec.extension_dir, "gem_make.out")) - expect(gem_make_out).not_to include("make -j8") end it "uses 3 slots from the available pool when running the compilation of an extension", rubygems: ">= 4.1.0.dev" do ENV.delete("MAKEFLAGS") - install_gemfile(<<~G, env: { "BUNDLE_JOBS" => "8" }) + install_gemfile(<<~G, env: { "BUNDLE_JOBS" => "8" }, raise_on_error: false) source "https://gem.repo4" gem "mypsych" G - gem_make_out = File.read(File.join(@gemspec.extension_dir, "gem_make.out")) - expect(gem_make_out).to include("make -j3") end it "consumes 3 slots from the pool when BUNDLE_JOBS isn't set", rubygems: ">= 4.1.0.dev" do ENV.delete("MAKEFLAGS") - install_gemfile(<<~G) + install_gemfile(<<~G, raise_on_error: false) source "https://gem.repo4" gem "mypsych" G - gem_make_out = File.read(File.join(@gemspec.extension_dir, "gem_make.out")) - expect(gem_make_out).to include("make -j3") end end diff --git a/spec/install/gemfile/git_spec.rb b/spec/install/gemfile/git_spec.rb index 2b74aa849ac8..847e5fa35efc 100644 --- a/spec/install/gemfile/git_spec.rb +++ b/spec/install/gemfile/git_spec.rb @@ -536,4 +536,33 @@ end end end + + describe "a git gem whose extension fails to build" do + # Where a build log goes is decided by the RubyGems running the install, and + # older ones write a bare gem_make.out into the extension directory. + it "keeps the build log with that checkout instead of the shared repository", rubygems: ">= 4.1.0.dev" do + build_git "foo", "1.0" do |s| + s.add_c_extension + # Overwrite the source add_c_extension wrote, before the checkout is + # committed, so that building it fails. + s.write "ext/foo.c", "#error forced build failure for test\n" + end + + install_gemfile <<~G, raise_on_error: false + source "https://gem.repo1" + gem "foo", :git => "#{lib_path("foo-1.0")}" + G + + # The log lands in the checkout's own extension directory, which is unique + # per revision and which `bundle clean` prunes along with the checkout. + logs = Dir.glob("#{Gem.dir}/bundler/gems/extensions/*/*/*/*.gem_make.out") + + expect(logs.size).to eq(1) + expect(File.basename(File.dirname(logs.first))).to start_with("foo-1.0-") + expect(File.read(logs.first)).to include("forced build failure for test") + + # Nothing is left directly under bundler/gems, which holds checkouts. + expect(Pathname.new("#{Gem.dir}/bundler/gems/build_info")).not_to exist + end + end end diff --git a/test/rubygems/test_gem_commands_install_command.rb b/test/rubygems/test_gem_commands_install_command.rb index a99afbfef74e..1127d0a85967 100644 --- a/test/rubygems/test_gem_commands_install_command.rb +++ b/test/rubygems/test_gem_commands_install_command.rb @@ -1721,6 +1721,9 @@ def test_pass_down_the_job_option_to_make write_file(extconf_path) do |io| io.puts "require 'mkmf'" + # Force the build to fail at the make stage so the build log is + # written. The make command line (including -j) is recorded there. + io.puts "File.write('a.c', '#error forced build failure for test')" io.puts "create_makefile '#{spec.name}'" end @@ -1729,12 +1732,12 @@ def test_pass_down_the_job_option_to_make end use_ui @ui do - assert_raise Gem::MockGemUi::SystemExitException, @ui.error do + assert_raise Gem::MockGemUi::TermError, @ui.error do @cmd.invoke "a", "-j4" end end - gem_make_out = File.read(File.join(gemspec.extension_dir, "gem_make.out")) + gem_make_out = File.read(File.join(gemspec.build_info_dir, "#{gemspec.full_name}.gem_make.out")) if vc_windows? && nmake_found? refute_includes(gem_make_out, " -j4") else diff --git a/test/rubygems/test_gem_commands_update_command.rb b/test/rubygems/test_gem_commands_update_command.rb index 9d15406bd13f..1056b4985ba3 100644 --- a/test/rubygems/test_gem_commands_update_command.rb +++ b/test/rubygems/test_gem_commands_update_command.rb @@ -856,6 +856,9 @@ def test_pass_down_the_job_option_to_make write_file(extconf_path) do |io| io.puts "require 'mkmf'" + # Force the build to fail at the make stage so the build log is + # written. The make command line (including -j) is recorded there. + io.puts "File.write('a.c', '#error forced build failure for test')" io.puts "create_makefile '#{spec.name}'" end @@ -869,7 +872,7 @@ def test_pass_down_the_job_option_to_make @cmd.invoke("a", "-j2") end - gem_make_out = File.read(File.join(gemspec.extension_dir, "gem_make.out")) + gem_make_out = File.read(File.join(gemspec.build_info_dir, "#{gemspec.full_name}.gem_make.out")) if vc_windows? && nmake_found? refute_includes(gem_make_out, " -j2") else diff --git a/test/rubygems/test_gem_doctor.rb b/test/rubygems/test_gem_doctor.rb index 1bcdc39022a8..705bcf09b729 100644 --- a/test/rubygems/test_gem_doctor.rb +++ b/test/rubygems/test_gem_doctor.rb @@ -121,6 +121,33 @@ def test_doctor_dry_run assert_equal Gem.path, [@gemhome, @userhome] end + def test_doctor_keeps_build_logs_of_installed_gems + a = gem "a" + + Gem.use_paths @userhome, @gemhome + + build_info_dir = File.join @gemhome, "build_info" + FileUtils.mkdir_p build_info_dir + + kept = ["#{a.full_name}.mkmf.log", "#{a.full_name}.gem_make.out"].map do |name| + File.join build_info_dir, name + end + stale = File.join build_info_dir, "b-2.gem_make.out" + + FileUtils.touch kept + [stale] + + doctor = Gem::Doctor.new @gemhome + + capture_output do + use_ui @ui do + doctor.doctor + end + end + + kept.each {|path| assert_path_exist path } + assert_path_not_exist stale + end + def test_doctor_non_gem_home other_dir = File.join @tempdir, "other", "dir" diff --git a/test/rubygems/test_gem_ext_builder.rb b/test/rubygems/test_gem_ext_builder.rb index 8f90687ede30..6b4eed2cf211 100644 --- a/test/rubygems/test_gem_ext_builder.rb +++ b/test/rubygems/test_gem_ext_builder.rb @@ -244,7 +244,9 @@ def test_build_extensions assert_path_exist @spec.extension_dir assert_path_exist @spec.gem_build_complete_path - assert_path_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join @spec.extension_dir, "mkmf.log" + assert_path_not_exist File.join @spec.gem_dir, "ext", "mkmf.log" assert_path_exist File.join @spec.extension_dir, "a.rb" assert_path_exist File.join @spec.gem_dir, "lib", "a.rb" assert_path_exist File.join @spec.gem_dir, "lib", "a", "b.rb" @@ -298,7 +300,9 @@ def test_build_extensions_install_ext_only assert_path_exist @spec.extension_dir assert_path_exist @spec.gem_build_complete_path - assert_path_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join @spec.extension_dir, "mkmf.log" + assert_path_not_exist File.join @spec.gem_dir, "ext", "mkmf.log" assert_path_exist File.join @spec.extension_dir, "a.rb" assert_path_not_exist File.join @spec.gem_dir, "lib", "a.rb" assert_path_not_exist File.join @spec.gem_dir, "lib", "a", "b.rb" @@ -351,13 +355,213 @@ def test_build_multiple_extensions assert_path_exist @spec.extension_dir assert_path_exist @spec.gem_build_complete_path assert_path_exist File.join @spec.gem_dir, "ext", "foo" - assert_path_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join @spec.extension_dir, "mkmf.log" + assert_path_not_exist File.join @spec.gem_dir, "ext", "mkmf.log" assert_path_exist File.join @spec.extension_dir, "a.rb" assert_path_exist File.join @spec.gem_dir, "lib", "a.rb" assert_path_exist File.join @spec.gem_dir, "lib", "a", "b.rb" end end + def test_build_extensions_does_not_install_logs_on_success + pend "terminates on mswin" if vc_windows? && ruby_repo? + + @spec.extensions << "ext/extconf.rb" + + ext_dir = File.join @spec.gem_dir, "ext" + FileUtils.mkdir_p ext_dir + + File.open File.join(ext_dir, "extconf.rb"), "w" do |f| + f.write <<-'RUBY' + require 'mkmf' + + create_makefile 'a' + RUBY + end + + use_ui @ui do + @builder.build_extensions + end + + assert_path_exist @spec.gem_build_complete_path + + # No build logs are left anywhere in the installation tree. + assert_path_not_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join @spec.extension_dir, "mkmf.log" + assert_path_not_exist File.join ext_dir, "mkmf.log" + assert_path_not_exist File.join ext_dir, "gem_make.out" + assert_path_not_exist File.join @spec.build_info_dir, "#{@spec.full_name}.mkmf.log" + assert_path_not_exist File.join @spec.build_info_dir, "#{@spec.full_name}.gem_make.out" + end + + def test_build_extensions_logs_to_build_info_on_make_failure + pend "terminates on mswin" if vc_windows? && ruby_repo? + + @spec.extensions << "ext/extconf.rb" + + ext_dir = File.join @spec.gem_dir, "ext" + FileUtils.mkdir_p ext_dir + + # have_header makes mkmf actually write an mkmf.log. extconf then succeeds, + # so "make clean" runs before the build and would delete that log unless it + # has been parked out of the way first. + File.open File.join(ext_dir, "extconf.rb"), "w" do |f| + f.write <<-'RUBY' + require 'mkmf' + + have_header 'stdio.h' + + File.write 'a.c', "#error forced build failure for test\n" + + create_makefile 'a' + RUBY + end + + e = assert_raise Gem::Ext::BuildError do + use_ui @ui do + @builder.build_extensions + end + end + + mkmf_log = File.join @spec.build_info_dir, "#{@spec.full_name}.mkmf.log" + + assert_path_exist mkmf_log + assert_includes e.message, mkmf_log + + assert_path_not_exist File.join @spec.extension_dir, "mkmf.log" + assert_path_not_exist File.join ext_dir, "mkmf.log" + end + + def test_build_extensions_removes_stale_build_info_logs_on_success + pend "terminates on mswin" if vc_windows? && ruby_repo? + + @spec.extensions << "ext/extconf.rb" + + ext_dir = File.join @spec.gem_dir, "ext" + FileUtils.mkdir_p ext_dir + + File.open File.join(ext_dir, "extconf.rb"), "w" do |f| + f.write <<-'RUBY' + require 'mkmf' + + create_makefile 'a' + RUBY + end + + # Logs left in build_info by an earlier failed build, and in the extension + # directory by a RubyGems old enough to install them there. + FileUtils.mkdir_p @spec.build_info_dir + FileUtils.mkdir_p @spec.extension_dir + stale = [ + File.join(@spec.build_info_dir, "#{@spec.full_name}.mkmf.log"), + File.join(@spec.build_info_dir, "#{@spec.full_name}.gem_make.out"), + File.join(@spec.extension_dir, "gem_make.out"), + ] + FileUtils.touch stale + + use_ui @ui do + @builder.build_extensions + end + + assert_path_exist @spec.gem_build_complete_path + + stale.each {|path| assert_path_not_exist path } + end + + def test_build_extensions_keeps_logs_when_no_makefile_is_generated + pend "terminates on mswin" if vc_windows? && ruby_repo? + + @spec.extensions << "ext/extconf.rb" + + ext_dir = File.join @spec.gem_dir, "ext" + FileUtils.mkdir_p ext_dir + + # extconf exits cleanly but generates no Makefile, the way one that bails out + # early on an unsupported platform does. The extension is then skipped rather + # than built, and the log is the only record of why. + File.open File.join(ext_dir, "extconf.rb"), "w" do |f| + f.write "# nothing to build on this platform\n" + end + + use_ui @ui do + @builder.build_extensions + end + + gem_make_out = File.join @spec.build_info_dir, "#{@spec.full_name}.gem_make.out" + + assert_path_exist gem_make_out + assert_includes File.read(gem_make_out), "no Makefile was found" + + # Skipping still leaves nothing behind in the installation tree. + assert_path_not_exist File.join @spec.extension_dir, "mkmf.log" + assert_path_not_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join ext_dir, "mkmf.log" + end + + def test_build_extensions_reports_build_error_when_logs_cannot_be_written + @spec.extensions << "extconf.rb" + + FileUtils.mkdir_p @spec.gem_dir + + # A plain file where build_info belongs makes every log write fail. + FileUtils.rm_rf @spec.build_info_dir + File.write @spec.build_info_dir, "" + + e = assert_raise Gem::Ext::BuildError do + use_ui @ui do + @builder.build_extensions + end + end + + # The build failure survives instead of being replaced by the log failure. + assert_match(/\AERROR: Failed to build gem native extension.$/, e.message) + assert_match(/: No such file/, e.message) + refute_includes e.message, "Results logged to" + end + + def test_build_extensions_logs_to_build_info_on_failure + pend "terminates on mswin" if vc_windows? && ruby_repo? + + @spec.extensions << "ext/extconf.rb" + + ext_dir = File.join @spec.gem_dir, "ext" + FileUtils.mkdir_p ext_dir + + File.open File.join(ext_dir, "extconf.rb"), "w" do |f| + f.write <<-'RUBY' + require 'mkmf' + + have_library 'nonexistent' or abort 'need libnonexistent' + + create_makefile 'a' + RUBY + end + + e = assert_raise Gem::Ext::BuildError do + use_ui @ui do + @builder.build_extensions + end + end + + mkmf_log = File.join @spec.build_info_dir, "#{@spec.full_name}.mkmf.log" + gem_make_out = File.join @spec.build_info_dir, "#{@spec.full_name}.gem_make.out" + + assert_path_exist mkmf_log + assert_path_exist gem_make_out + + # Logs are not left in the installation tree. + assert_path_not_exist File.join @spec.extension_dir, "mkmf.log" + assert_path_not_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist File.join ext_dir, "mkmf.log" + + # The error message points at the new build_info paths. + assert_includes e.message, gem_make_out + assert_includes e.message, mkmf_log + + assert_path_not_exist @spec.gem_build_complete_path + end + def test_build_extensions_none use_ui @ui do @builder.build_extensions @@ -401,12 +605,14 @@ def test_build_extensions_extconf_bad assert_equal "Building native extensions. This could take a while...\n", @ui.output assert_equal "", @ui.error - gem_make_out = File.join @spec.extension_dir, "gem_make.out" + gem_make_out = File.join @spec.build_info_dir, "#{@spec.full_name}.gem_make.out" cmd_make_out = File.read(gem_make_out) assert_match %r{#{Regexp.escape Gem.ruby} .* extconf\.rb}, cmd_make_out assert_match(/: No such file/, cmd_make_out) + assert_path_not_exist File.join @spec.extension_dir, "gem_make.out" + assert_path_not_exist @spec.gem_build_complete_path assert_equal cwd, Dir.pwd @@ -414,7 +620,7 @@ def test_build_extensions_extconf_bad def test_build_extensions_unsupported FileUtils.mkdir_p @spec.gem_dir - gem_make_out = File.join @spec.extension_dir, "gem_make.out" + gem_make_out = File.join @spec.build_info_dir, "#{@spec.full_name}.gem_make.out" @spec.extensions << nil e = assert_raise Gem::Ext::BuildError do diff --git a/test/rubygems/test_gem_ext_ext_conf_builder.rb b/test/rubygems/test_gem_ext_ext_conf_builder.rb index bc383e5540a9..b776883f4557 100644 --- a/test/rubygems/test_gem_ext_ext_conf_builder.rb +++ b/test/rubygems/test_gem_ext_ext_conf_builder.rb @@ -26,16 +26,18 @@ def test_class_build output = [] - result = Gem::Ext::ExtConfBuilder.build "extconf.rb", @dest_path, output, [], nil, @ext - - assert_same result, output - - assert_match(/^current directory:/, output[0]) - assert_match(/^#{Regexp.quote(Gem.ruby)}.* extconf.rb/, output[1]) - if Gem.java_platform? - assert_includes(output, "Skipping make for extconf.rb as no Makefile was found.") + # extconf returns before creating a Makefile, so the extension is skipped. + # Deciding what that means is Gem::Ext::Builder#build_extension's job now, + # so the error reaches it instead of being swallowed here. + assert_raise Gem::Ext::Builder::NoMakefileError do + Gem::Ext::ExtConfBuilder.build "extconf.rb", @dest_path, output, [], nil, @ext + end else + result = Gem::Ext::ExtConfBuilder.build "extconf.rb", @dest_path, output, [], nil, @ext + + assert_same result, output + assert_equal "creating Makefile\n", output[2] assert_match(/^current directory:/, output[3]) assert_contains_make_command "clean", output[4] @@ -43,6 +45,9 @@ def test_class_build assert_contains_make_command "install", output[10] end + assert_match(/^current directory:/, output[0]) + assert_match(/^#{Regexp.quote(Gem.ruby)}.* extconf.rb/, output[1]) + assert_empty Dir.glob(File.join(@ext, "siteconf*.rb")) assert_empty Dir.glob(File.join(@ext, ".gem.*")) end @@ -110,10 +115,12 @@ def test_class_build_extconf_fail assert_equal "extconf failed, exit code 1", error.message assert_match(/^#{Regexp.quote(Gem.ruby)}.* extconf.rb/, output[1]) - assert_match(File.join(@dest_path, "mkmf.log"), output[4]) - assert_includes(output, "To see why this extension failed to compile, please check the mkmf.log which can be found here:\n") + refute_includes(output, "To see why this extension failed to compile, please check the mkmf.log which can be found here:\n") - assert_path_exist File.join @dest_path, "mkmf.log" + # mkmf.log is left in the extension directory; deciding where it ends up is + # left to Gem::Ext::Builder#build_extension. + assert_path_exist File.join @ext, "mkmf.log" + assert_path_not_exist File.join @dest_path, "mkmf.log" end def test_class_build_extconf_success_without_warning @@ -133,6 +140,9 @@ def test_class_build_extconf_success_without_warning refute_includes(output, "To see why this extension failed to compile, please check the mkmf.log which can be found here:\n") + # mkmf.log is parked in dest_path so that "make clean" cannot delete it. + # Dropping it is Gem::Ext::Builder#build_extension's job, not this one's. + assert_path_not_exist File.join @ext, "mkmf.log" assert_path_exist File.join @dest_path, "mkmf.log" end diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index c63e68be47dd..3c5d22ba7b55 100644 --- a/test/rubygems/test_gem_specification.rb +++ b/test/rubygems/test_gem_specification.rb @@ -1562,8 +1562,9 @@ def test_build_extensions_preview @ext.build_extensions + # A successful build no longer leaves gem_make.out in the install tree. gem_make_out = File.join @ext.extension_dir, "gem_make.out" - assert_path_exist gem_make_out + assert_path_not_exist gem_make_out end def test_contains_requirable_file_eh diff --git a/test/rubygems/test_gem_uninstaller.rb b/test/rubygems/test_gem_uninstaller.rb index d1aacb40536b..2e526e157757 100644 --- a/test/rubygems/test_gem_uninstaller.rb +++ b/test/rubygems/test_gem_uninstaller.rb @@ -391,6 +391,39 @@ def test_uninstall_extension assert_path_not_exist @spec.extension_dir end + def test_uninstall_removes_build_info_logs + @spec.extensions << "extconf.rb" + write_file File.join(@tempdir, "extconf.rb") do |io| + io.write <<-RUBY +require 'mkmf' +create_makefile '#{@spec.name}' + RUBY + end + + @spec.files += %w[extconf.rb] + + use_ui @ui do + path = Gem::Package.build @spec + + installer = Gem::Installer.at path, force: true + installer.install + end + + # Build logs left behind in build_info by a previous failed build. + FileUtils.mkdir_p @spec.build_info_dir + mkmf_log = File.join @spec.build_info_dir, "#{@spec.full_name}.mkmf.log" + gem_make_out = File.join @spec.build_info_dir, "#{@spec.full_name}.gem_make.out" + FileUtils.touch mkmf_log + FileUtils.touch gem_make_out + + uninstaller = Gem::Uninstaller.new @spec.name, executables: true + uninstaller.uninstall + + assert_path_not_exist @spec.extension_dir + assert_path_not_exist mkmf_log + assert_path_not_exist gem_make_out + end + def test_uninstall_nonexistent uninstaller = Gem::Uninstaller.new "bogus", executables: true