diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index b50de2ddde128a..72b3c200edb4fb 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -98,7 +98,7 @@ jobs: rustup install ${{ matrix.rust_version }} --profile minimal rustup default ${{ matrix.rust_version }} - - uses: taiki-e/install-action@0758d235715de2f3551eacc980d9ae8fce9342c3 # v2.87.3 + - uses: taiki-e/install-action@5bf6ce016fd2e72eefc647cbca1e4213f65955b8 # v2.87.5 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 1471a0d88e7c92..b0475e62dab611 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -152,7 +152,7 @@ jobs: ruby-version: '3.1' bundler: none - - uses: taiki-e/install-action@0758d235715de2f3551eacc980d9ae8fce9342c3 # v2.87.3 + - uses: taiki-e/install-action@5bf6ce016fd2e72eefc647cbca1e4213f65955b8 # v2.87.5 with: tool: nextest@0.9 if: ${{ matrix.test_task == 'zjit-check' }} diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb index 1b9badf0226325..42c7093c82fa4c 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -96,7 +96,8 @@ def call handle_error if failed_specs.any? @specs ensure - worker_pool&.stop + @worker_pool&.stop + @download_worker_pool&.stop end private @@ -166,17 +167,18 @@ def install_serially end end + def download_worker_pool + @download_worker_pool ||= Bundler::Worker.new(@size, "Gem Downloader", + ->(spec_install, worker_num) { do_download(spec_install, worker_num) }, response_queue: response_queue) + end + def worker_pool - @worker_pool ||= Bundler::Worker.new @size, "Parallel Installer", lambda {|spec_install, worker_num| - case spec_install.state - when :enqueued - do_download(spec_install, worker_num) - when :installable - do_install(spec_install, worker_num) - else - spec_install - end - } + @worker_pool ||= Bundler::Worker.new(@size, "Parallel Installer", + ->(spec_install, worker_num) { do_install(spec_install, worker_num) }, response_queue: response_queue) + end + + def response_queue + @response_queue ||= Thread::Queue.new end def do_download(spec_install, worker_num) @@ -214,24 +216,24 @@ def do_install(spec_install, worker_num) spec_install end - # Dequeue a spec and save its post-install message and then enqueue the - # remaining specs. - # Some specs might've had to wait til this spec was installed to be - # processed so the call to `enqueue_specs` is important after every - # dequeue. + # Process one completed download or installation. Downloads can finish + # before their dependencies are installed, so check all downloaded specs + # after each completion and enqueue any that are now installable. def process_specs(installed_specs) spec = worker_pool.deq if spec.installed? installed_specs[spec.name] = true - return elsif spec.failed? return - elsif spec.ready_to_install?(installed_specs) - spec.state = :installable end - worker_pool.enq(spec, priority: spec.enqueue_with_priority?) + @specs.each do |candidate| + next unless candidate.ready_to_install?(installed_specs) + + candidate.state = :installable + worker_pool.enq(candidate, priority: candidate.enqueue_with_priority?) + end end def finished_installing? @@ -270,11 +272,8 @@ def require_tree_for_spec(spec) t end - # Keys in the remains hash represent uninstalled gems specs. - # We enqueue all gem specs that do not have any dependencies. - # Later we call this lambda again to install specs that depended on - # previously installed specifications. We continue until all specs - # are installed. + # Queue every missing spec for download. `process_specs` schedules each + # downloaded spec for installation once its dependencies are installed. def enqueue_specs(installed_specs) @specs.each do |spec| if spec.installed? @@ -283,7 +282,7 @@ def enqueue_specs(installed_specs) end spec.state = :enqueued - worker_pool.enq spec + download_worker_pool.enq spec end end end diff --git a/lib/bundler/rubygems_ext.rb b/lib/bundler/rubygems_ext.rb index 730d1e522b4ce3..d3c4ce6d9dcf94 100644 --- a/lib/bundler/rubygems_ext.rb +++ b/lib/bundler/rubygems_ext.rb @@ -281,6 +281,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/bundler/source/rubygems.rb b/lib/bundler/source/rubygems.rb index 897968b3e11bd0..41eb85f5fc8f2e 100644 --- a/lib/bundler/source/rubygems.rb +++ b/lib/bundler/source/rubygems.rb @@ -25,6 +25,7 @@ def initialize(options = {}) @checksum_store = Checksum::Store.new @gem_installers = {} @gem_installers_mutex = Mutex.new + @remote_spec_for_mutex = Mutex.new @remote_specs_mutex = Mutex.new cooldown = options["cooldown"] @@ -443,12 +444,14 @@ def remote_specs # Looks up a single spec in the remote sources, fetching only its own # name when the full remote index is not already materialized. def remote_spec_for(spec) - return remote_specs.search(spec).first if @remote_specs || api_fetchers.empty? + @remote_spec_for_mutex.synchronize do + return remote_specs.search(spec).first if @remote_specs || api_fetchers.empty? - index = Index.build do |idx| - fetch_names(api_fetchers, [spec.name], idx) + index = Index.build do |idx| + fetch_names(api_fetchers, [spec.name], idx) + end + index.search(spec).first end - index.search(spec).first end def fetch_names(fetchers, dependency_names, index) diff --git a/lib/bundler/worker.rb b/lib/bundler/worker.rb index 77f4f004aa6961..f4e99de305d810 100644 --- a/lib/bundler/worker.rb +++ b/lib/bundler/worker.rb @@ -19,11 +19,12 @@ def initialize(exn) # @param size [Integer] Size of pool # @param name [String] name the name of the worker # @param func [Proc] job to run in inside the worker pool - def initialize(size, name, func) + # @param response_queue [Thread::Queue] queue that receives completed jobs + def initialize(size, name, func, response_queue: Thread::Queue.new) @name = name @request_queue = Thread::Queue.new @request_queue_with_priority = Thread::Queue.new - @response_queue = Thread::Queue.new + @response_queue = response_queue @func = func @size = size @threads = nil diff --git a/lib/rubygems/commands/install_command.rb b/lib/rubygems/commands/install_command.rb index 2ebbc40a03080f..288da7c31feee7 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 114946209b4bcb..774aa2c1864669 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" @@ -117,7 +117,7 @@ def doctor_child(sub_directory, extension) # :nodoc: if sub_directory == "specifications" && File.directory?(child) && Gem::ContentAddress.valid_ruby_abi?(ent) - doctor_child(File.join(sub_directory, ent), extension) if ent == Gem.ruby_abi && !File.symlink?(child) + doctor_child(File.join(sub_directory, ent), *extensions) if ent == Gem.ruby_abi && !File.symlink?(child) next end @@ -135,4 +135,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 f1fce48823a888..b7e80d6b06bb89 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 822454355d104d..9cf7eafc5dc344 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 28bf33ea710605..f87ee2f4984167 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/bundler/installer/parallel_installer_spec.rb b/spec/bundler/bundler/installer/parallel_installer_spec.rb index 20d39e885be3d8..8112660af35a4d 100644 --- a/spec/bundler/bundler/installer/parallel_installer_spec.rb +++ b/spec/bundler/bundler/installer/parallel_installer_spec.rb @@ -56,21 +56,19 @@ end let(:installer) { Bundler::Installer.new(bundled_app, definition) } - it "queues native extensions in priority" do + it "prioritizes native extensions for installation" do parallel_installer = Bundler::ParallelInstaller.new(installer, definition.specs, 2, false, true) - worker_pool = parallel_installer.send(:worker_pool) - expected = 6 # Enqueue to download bundler and the 2 gems. Enqueue to install Bundler and the 2 gems. - - expect(worker_pool).to receive(:enq).exactly(expected).times.and_wrap_original do |original_enq, spec, opts| - unless opts.nil? # Enqueued for download, no priority - if spec.name == "gem_with_extension" - expect(opts).to eq({ priority: true }) - else - expect(opts).to eq({ priority: false }) - end + download_worker_pool = parallel_installer.send(:download_worker_pool) + install_worker_pool = parallel_installer.send(:worker_pool) + + expect(download_worker_pool).to receive(:enq).exactly(3).times.and_call_original + expect(install_worker_pool).to receive(:enq).exactly(3).times.and_wrap_original do |original_enq, spec, opts| + if spec.name == "gem_with_extension" + expect(opts).to eq({ priority: true }) + else + expect(opts).to eq({ priority: false }) end - opts ||= {} original_enq.call(spec, **opts) end @@ -78,6 +76,42 @@ end end + describe "worker pools" do + it "uses separate worker pools with the configured size" do + parallel_installer = described_class.new(nil, [], 2, false, false) + + download_worker_pool = parallel_installer.send(:download_worker_pool) + install_worker_pool = parallel_installer.send(:worker_pool) + + expect(download_worker_pool).not_to equal(install_worker_pool) + expect(download_worker_pool.instance_variable_get(:@size)).to eq(2) + expect(install_worker_pool.instance_variable_get(:@size)).to eq(2) + ensure + install_worker_pool&.stop + download_worker_pool&.stop + end + + it "restores the previous interrupt handler after shutting down" do + parallel_installer = described_class.new(nil, [], 2, false, false) + download_worker_pool = parallel_installer.send(:download_worker_pool) + install_worker_pool = parallel_installer.send(:worker_pool) + + previous_handler = Signal.trap("INT", "IGNORE") + custom_handler = proc {} + Signal.trap("INT", custom_handler) + + download_worker_pool.send(:create_threads) + install_worker_pool.send(:create_threads) + + parallel_installer.call + + restored_handler = Signal.trap("INT", previous_handler) + expect(restored_handler).to equal(custom_handler) + ensure + Signal.trap("INT", previous_handler) if previous_handler + end + end + describe "connect to make jobserver" do before do unless Gem::Installer.private_method_defined?(:build_jobs) @@ -152,34 +186,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 +228,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/bundler/bundler/worker_spec.rb b/spec/bundler/bundler/worker_spec.rb index 2ad2845e378c26..7daf16cfdd2c7a 100644 --- a/spec/bundler/bundler/worker_spec.rb +++ b/spec/bundler/bundler/worker_spec.rb @@ -18,6 +18,24 @@ expect { subject.enq "a" }.to raise_error(Bundler::ThreadCreationError, "Failed to create threads for the Spec Worker worker: error creating thread") end end + + context "with a shared response queue" do + it "allows workers to publish to the same queue" do + response_queue = Thread::Queue.new + workers = [ + described_class.new(1, "First", function, response_queue: response_queue), + described_class.new(1, "Second", function, response_queue: response_queue), + ] + + workers.first.enq("first") + workers.last.enq("second") + + responses = Array.new(2) { workers.first.deq.first } + expect(responses).to contain_exactly("first", "second") + ensure + workers&.reverse_each(&:stop) + end + end end describe "priority queue" do diff --git a/spec/bundler/commands/install_spec.rb b/spec/bundler/commands/install_spec.rb index a0b56a3d2970a4..d4b88902e21e28 100644 --- a/spec/bundler/commands/install_spec.rb +++ b/spec/bundler/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/bundler/install/gemfile/git_spec.rb b/spec/bundler/install/gemfile/git_spec.rb index 2b74aa849ac8fc..847e5fa35efc03 100644 --- a/spec/bundler/install/gemfile/git_spec.rb +++ b/spec/bundler/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/ruby/test_box.rb b/test/ruby/test_box.rb index b3ecd0c546394d..647eb960da915f 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -1195,6 +1195,79 @@ def foo_box = Ruby::Box.current end; end + def test_symbol_to_proc_with_escaped_binding + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + # Regression test for [BUG] Local ep without cme/box, flags: 66660087. + # binding() may escape TOP env and propagate LOCAL to IFUNC frames. + assert_nothing_raised do + using Module.new { + refine ::Binding do + def eval_methods + ::Kernel.instance_method(:methods).bind_call(receiver) + end + end + } + + result = binding.eval_methods.map(&:to_s) + assert_kind_of Array, result + assert result.all? { |x| x.is_a?(String) } + end + end; + end + + def test_method_to_proc_called_from_ruby_frame + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + # Regression test for [BUG] Local ep without cme/box, flags: 77770021. + # An ifunc proc invoked directly from a Ruby frame (e.g. Proc#call) + # has no enclosing CFUNC frame, so the caller Ruby frame determines + # the box. + assert_nothing_raised do + method(:require).to_proc.call("English") + end + assert_equal 1, $LOADED_FEATURES.grep(/English/).size + end; + end + + def test_symbol_to_proc_uses_current_box + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + box = Ruby::Box.new + + normal, via_sym_proc = box.eval(<<~'RUBY') + class Array + def box_only_method + :ok + end + end + + normal = [[1]].flat_map { |ary| ary.box_only_method } + via_sym_proc = [[1]].flat_map(&:box_only_method) + [normal, via_sym_proc] + RUBY + + assert_equal [:ok], normal + assert_equal [:ok], via_sym_proc + + assert_raise(NoMethodError) { [1].box_only_method } + end; + end + + def test_symbol_to_proc_uses_main_box + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + class Array + def main_box_only_method + :ok + end + end + + assert_equal [:ok], [[1]].flat_map { |ary| ary.main_box_only_method } + assert_equal [:ok], [[1]].flat_map(&:main_box_only_method) + end; + end + def test_very_basic_method_calls_and_constants assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; diff --git a/test/rubygems/helper.rb b/test/rubygems/helper.rb index 4392fe583a72c5..09b7f427ebbf1f 100644 --- a/test/rubygems/helper.rb +++ b/test/rubygems/helper.rb @@ -20,6 +20,8 @@ skip ".gemspec" end + SimpleCov.print_error_status = false + # Prevent SimpleCov from running in subprocesses spawned by assert_separately ENV["SIMPLECOV_SUBPROCESS"] = "1" end @@ -1379,6 +1381,15 @@ def util_set_RUBY_VERSION(version, patchlevel, revision, description, engine = " Object.const_set :RUBY_ENGINE_VERSION, engine_version end + ## + # Pins the running Ruby to the first release of +ruby_abi+, so that the + # "~> X.Y.0" requirement a content addressed gem pins its ABI with is + # satisfied on a prerelease Ruby too. Pair with util_restore_RUBY_VERSION. + + def util_pin_ruby_to_abi(ruby_abi) + util_set_RUBY_VERSION "#{ruby_abi}.0", 0, RUBY_REVISION, "ruby #{ruby_abi}.0" + end + def util_restore_RUBY_VERSION util_clear_RUBY_VERSION diff --git a/test/rubygems/test_gem_commands_install_command.rb b/test/rubygems/test_gem_commands_install_command.rb index 01786bea58cb89..1b8978670462c8 100644 --- a/test/rubygems/test_gem_commands_install_command.rb +++ b/test/rubygems/test_gem_commands_install_command.rb @@ -1740,6 +1740,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 @@ -1748,12 +1751,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 fc42baa4a7bf70..1449f5425f4efe 100644 --- a/test/rubygems/test_gem_commands_update_command.rb +++ b/test/rubygems/test_gem_commands_update_command.rb @@ -954,6 +954,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 @@ -967,7 +970,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_dependency_installer.rb b/test/rubygems/test_gem_dependency_installer.rb index 685455068f6487..95d455ad5f3d7b 100644 --- a/test/rubygems/test_gem_dependency_installer.rb +++ b/test/rubygems/test_gem_dependency_installer.rb @@ -425,8 +425,8 @@ def test_install_local end def test_install_local_by_name_preserves_content_address - ruby_abi = Gem.ruby_version.segments.first(2).join(".") - util_set_RUBY_VERSION "#{ruby_abi}.0", 0, RUBY_REVISION, "ruby #{ruby_abi}.0" + ruby_abi = Gem.ruby_abi + util_pin_ruby_to_abi ruby_abi _spec, ca_gem = util_gem("ca", "1.0.0", ruby_abi: ruby_abi) do |spec| spec.platform = Gem::Platform.local end diff --git a/test/rubygems/test_gem_doctor.rb b/test/rubygems/test_gem_doctor.rb index 9fd6f33641e21c..1554e7af128dd4 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" @@ -228,10 +255,13 @@ def test_doctor_removes_corrupt_abi_scoped_gemspec doctor = Gem::Doctor.new @gemhome - use_ui @ui do - doctor.doctor + _, err = capture_output do + use_ui @ui do + doctor.doctor + end end + assert_include err, "Invalid gemspec" assert_path_exist abi_dir assert_path_exist gemspec_path assert_path_not_exist corrupt_path @@ -270,10 +300,13 @@ def test_doctor_does_not_recurse_into_abi_symlink doctor = Gem::Doctor.new @gemhome - use_ui @ui do - doctor.doctor + _, err = capture_output do + use_ui @ui do + doctor.doctor + end end + assert_include err, "Invalid gemspec" assert File.symlink?(link) assert_path_exist outside_path end diff --git a/test/rubygems/test_gem_ext_builder.rb b/test/rubygems/test_gem_ext_builder.rb index 8f90687ede306f..6b4eed2cf21175 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 bc383e5540a9e3..b776883f45571d 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_package_task.rb b/test/rubygems/test_gem_package_task.rb index f03af2a3a39f0d..e180cc5d4ce7ec 100644 --- a/test/rubygems/test_gem_package_task.rb +++ b/test/rubygems/test_gem_package_task.rb @@ -44,6 +44,9 @@ def test_gem_package end def test_moves_filename_returned_by_build + original_rake_fileutils_verbosity = RakeFileUtils.verbose_flag + RakeFileUtils.verbose_flag = false + gem = Gem::Specification.new do |g| g.name = "pkgr" g.version = "1.2.3" @@ -82,6 +85,8 @@ def test_moves_filename_returned_by_build assert_equal "pkg/pkgr-1.2.3-01234567.gem", built_files.first assert_path_not_exist "pkg/pkgr-1.2.3-arm64-darwin.gem" end + ensure + RakeFileUtils.verbose_flag = original_rake_fileutils_verbosity end def test_gem_package_prints_to_stdout_by_default diff --git a/test/rubygems/test_gem_resolver.rb b/test/rubygems/test_gem_resolver.rb index 8e17d5c1107980..9fff107e02d366 100644 --- a/test/rubygems/test_gem_resolver.rb +++ b/test/rubygems/test_gem_resolver.rb @@ -338,7 +338,8 @@ def test_prefers_content_addressed_gem_for_same_platform def test_prefers_compatible_content_addressed_gem_over_more_specific_platform util_set_arch "arm64-darwin-27" - current_abi = "#{Gem.ruby_version.segments[0]}.#{Gem.ruby_version.segments[1]}" + current_abi = Gem.ruby_abi + util_pin_ruby_to_abi current_abi ca_spec = util_spec "a", "1" fat_spec = util_spec "a", "1" @@ -352,6 +353,8 @@ def test_prefers_compatible_content_addressed_gem_over_more_specific_platform dependency = make_dep "a" resolver = Gem::Resolver.new([dependency], s) assert_resolves_to [ca_spec], resolver + ensure + util_restore_RUBY_VERSION end def test_prefers_more_specific_platform_over_content_addressed_gem_for_another_ruby @@ -419,7 +422,8 @@ def test_falls_back_to_non_content_addressable_before_source_when_content_addres end def test_prefers_compatible_content_addressed_gem_when_multiple_abis_available - current_abi = "#{Gem.ruby_version.segments[0]}.#{Gem.ruby_version.segments[1]}" + current_abi = Gem.ruby_abi + util_pin_ruby_to_abi current_abi ca_compatible = util_spec "a", "1" ca_incompatible = util_spec "a", "1" @@ -435,6 +439,8 @@ def test_prefers_compatible_content_addressed_gem_when_multiple_abis_available dependency = make_dep "a" resolver = Gem::Resolver.new([dependency], s) assert_resolves_to [ca_compatible], resolver + ensure + util_restore_RUBY_VERSION end def test_raises_when_only_content_addressed_gem_is_incompatible diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index 025fec7e66ebfd..dc32a62907863c 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 137bb1de97a9c5..ea2ac081012b1d 100644 --- a/test/rubygems/test_gem_uninstaller.rb +++ b/test/rubygems/test_gem_uninstaller.rb @@ -419,6 +419,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 diff --git a/vm.c b/vm.c index 5454bff570e974..23bdefdabd5527 100644 --- a/vm.c +++ b/vm.c @@ -118,54 +118,63 @@ VM_EP_RUBY_LEP(const rb_execution_context_t *ec, const rb_control_frame_t *curre const rb_control_frame_t * const eocfp = RUBY_VM_END_CONTROL_FRAME(ec); /* end of control frame pointer */ const rb_control_frame_t *cfp = current_cfp; - if (VM_ENV_FRAME_TYPE_P(ep, VM_FRAME_MAGIC_IFUNC)) { - ep = VM_EP_LEP(current_cfp->ep); - /** - * Returns CFUNC frame only in this case. - * - * Usually CFUNC frame doesn't represent the current box and it should operate - * the caller box. See the example: - * - * # in the main box - * module Kernel - * def foo = "foo" - * module_function :foo - * end - * - * In the case above, `module_function` is defined in the root box. - * If `module_function` worked in the root box, `Kernel#foo` is invisible - * from it and it causes NameError: undefined method `foo` for module `Kernel`. - * - * But in cases of IFUNC (blocks written in C), IFUNC doesn't have its own box - * and its local env frame will be CFUNC frame. - * For example, `Enumerator#chunk` calls IFUNC blocks, written as `chunk_i` function. - * - * [1].chunk{ it.even? }.each{ ... } - * - * Before calling the Ruby block `{ it.even? }`, `#chunk` calls `chunk_i` as IFUNC - * to iterate the array's members (it's just like `#each`). - * We expect that `chunk_i` works as expected by the implementation of `#chunk` - * without any overwritten definitions from boxes. - * So the definitions on IFUNC frames should be equal to the caller CFUNC. - */ - VM_ASSERT(VM_ENV_FRAME_TYPE_P(ep, VM_FRAME_MAGIC_CFUNC)); - return ep; - } + /** + * For IFUNC frames, returns the ep of the enclosing CFUNC frame. + * + * Usually CFUNC frame doesn't represent the current box and it should operate + * the caller box. See the example: + * + * # in the main box + * module Kernel + * def foo = "foo" + * module_function :foo + * end + * + * In the case above, `module_function` is defined in the root box. + * If `module_function` worked in the root box, `Kernel#foo` is invisible + * from it and it causes NameError: undefined method `foo` for module `Kernel`. + * + * But in cases of IFUNC (blocks written in C), IFUNC doesn't have its own box + * and its local env frame will be CFUNC frame. + * For example, `Enumerator#chunk` calls IFUNC blocks, written as `chunk_i` function. + * + * [1].chunk{ it.even? }.each{ ... } + * + * Before calling the Ruby block `{ it.even? }`, `#chunk` calls `chunk_i` as IFUNC + * to iterate the array's members (it's just like `#each`). + * We expect that `chunk_i` works as expected by the implementation of `#chunk` + * without any overwritten definitions from boxes. + * So the definitions on IFUNC frames should be equal to the caller CFUNC. + * + * NOTE: We traverse the cfp chain directly instead of using VM_EP_LEP. + * When an IFUNC env is escaped to the heap (e.g., due to a surrounding + * `binding` call), the env may acquire VM_ENV_FLAG_LOCAL, causing VM_EP_LEP + * to return the IFUNC ep itself rather than the enclosing CFUNC ep. + * + * NOTE: An IFUNC may also have no enclosing CFUNC frame at all, when an + * ifunc proc is invoked directly from Ruby code (e.g. Proc#call on a proc + * created by Method#to_proc). In that case the caller Ruby frame + * determines the box, so continue to the local ep walk below. + */ + while (VM_ENV_FRAME_TYPE_P(ep, VM_FRAME_MAGIC_IFUNC) || + VM_ENV_FRAME_TYPE_P(ep, VM_FRAME_MAGIC_CFUNC)) { + bool from_ifunc = VM_ENV_FRAME_TYPE_P(ep, VM_FRAME_MAGIC_IFUNC); - while (VM_ENV_FRAME_TYPE_P(ep, VM_FRAME_MAGIC_CFUNC)) { cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp); - - VM_BOX_ASSERT(cfp, "CFUNC should have a valid previous control frame"); - VM_BOX_ASSERT(cfp < eocfp, "CFUNC should have a valid caller frame"); - if (!cfp || cfp >= eocfp) { + VM_BOX_ASSERT(RUBY_VM_VALID_CONTROL_FRAME_P(cfp, eocfp), "Valid caller control frame expected"); + if (!RUBY_VM_VALID_CONTROL_FRAME_P(cfp, eocfp)) { return NULL; } - VM_BOX_ASSERT(cfp->ep, "CFUNC should have a valid caller frame with env"); + VM_BOX_ASSERT(cfp->ep, "Caller control frame should have a valid env"); ep = cfp->ep; if (!ep) { return NULL; } + + if (from_ifunc && VM_ENV_FRAME_TYPE_P(ep, VM_FRAME_MAGIC_CFUNC)) { + return ep; + } } while (!VM_ENV_LOCAL_P(ep)) { @@ -673,6 +682,8 @@ static void add_opt_method_entry(const rb_method_entry_t *me); #define VM_ASSERT_TYPE3(obj, type1, type2, type3) \ VM_ASSERT(RB_TYPE_3_P(obj, type1, type2, type3), #obj ": %s", rb_obj_info(obj)) +static const rb_box_t * current_box_on_cfp(const rb_execution_context_t *ec, const rb_control_frame_t *cfp); + #include "vm_insnhelper.c" #include "vm_exec.c" diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 0542221f58d33e..05c9bf75b7753f 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -5280,7 +5280,72 @@ rb_vm_yield_with_cfunc(rb_execution_context_t *ec, const struct rb_captured_bloc static VALUE vm_yield_with_symbol(rb_execution_context_t *ec, VALUE symbol, int argc, const VALUE *argv, int kw_splat, VALUE block_handler) { - return rb_sym_proc_call(SYM2ID(symbol), argc, argv, kw_splat, rb_vm_bh_to_procval(ec, block_handler)); + VALUE passed_proc = rb_vm_bh_to_procval(ec, block_handler); + + if (!rb_box_available()) { + return rb_sym_proc_call(SYM2ID(symbol), argc, argv, kw_splat, passed_proc); + } + + const rb_control_frame_t *reg_cfp = ec->cfp; + const rb_control_frame_t *ruby_cfp = rb_vm_get_ruby_level_next_cfp(ec, reg_cfp); + const rb_box_t *box = NULL; + + /* + * Traverse the frames until a user box (Main or Optional Box) is found. + * Frames for built-in methods like run in the Root or Master Box, + */ + while (ruby_cfp) { + box = current_box_on_cfp(ec, ruby_cfp); + if (BOX_USER_P(box)) { + break; + } + ruby_cfp = rb_vm_get_ruby_level_next_cfp(ec, RUBY_VM_PREVIOUS_CONTROL_FRAME(ruby_cfp)); + } + + // Fallback to the normal call if no user box is found + if (!ruby_cfp || !box) { + return rb_sym_proc_call(SYM2ID(symbol), argc, argv, kw_splat, passed_proc); + } + + /* + * Push a dummy block frame whose outer env is `ruby_cfp` so that the + * method resolves in the caller's box (via the ep chain) and the + * backtrace reads like a usual block invocation at the caller's site. + */ + const rb_iseq_t *caller_iseq = CFP_ISEQ(ruby_cfp); + VALUE name = rb_sprintf("block in %"PRIsVALUE, rb_iseq_label(caller_iseq)); + const rb_iseq_t *iseq = rb_iseq_new_with_opt(Qnil, name, + rb_iseq_path(caller_iseq), rb_iseq_realpath(caller_iseq), + rb_vm_get_sourceline(ruby_cfp), caller_iseq, 0, + ISEQ_TYPE_BLOCK, NULL, Qnil); + volatile VALUE val = Qnil; + enum ruby_tag_type state; + + vm_push_frame(ec, iseq, VM_FRAME_MAGIC_BLOCK | VM_FRAME_FLAG_FINISH, + ruby_cfp->self, VM_GUARDED_PREV_EP(ruby_cfp->ep), + Qfalse, /* cref or me */ + ISEQ_BODY(iseq)->iseq_encoded, reg_cfp->sp, + ISEQ_BODY(iseq)->local_table_size, ISEQ_BODY(iseq)->stack_max); + + /* + * The pushed frame is finished (owned by no vm_exec loop), so catch the + * non-local exit here, pop the frame, and let the caller's handlers see + * the exception. + */ + EC_PUSH_TAG(ec); + if ((state = EC_EXEC_TAG()) == TAG_NONE) { + val = rb_sym_proc_call(SYM2ID(symbol), argc, argv, kw_splat, passed_proc); + } + EC_POP_TAG(); + + if (state != TAG_NONE) { + rb_vm_rewind_cfp(ec, (rb_control_frame_t *)reg_cfp); + EC_JUMP_TAG(ec, state); + } + + rb_vm_pop_frame(ec); + + return val; } static inline int