Stop installing native extension build logs

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)
 

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 > \\

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

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

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

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
 

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)