From 90ae572983080b95047df8f7cf771c15658cb777 Mon Sep 17 00:00:00 2001
From: Sylvain Joyeux <sylvain.joyeux@tidewise.io>
Date: Wed, 30 Apr 2025 13:29:29 -0300
Subject: [PATCH 1/8] chore: remove explicit requires

---
 lib/autobuild.rb                    | 21 ++++++++++++++++++++-
 lib/autobuild/build_logfile.rb      |  2 --
 lib/autobuild/environment.rb        |  4 ----
 lib/autobuild/import/archive.rb     |  7 -------
 lib/autobuild/import/darcs.rb       |  4 ----
 lib/autobuild/import/git.rb         |  7 -------
 lib/autobuild/import/hg.rb          |  5 -----
 lib/autobuild/import/svn.rb         |  4 ----
 lib/autobuild/importer.rb           |  3 ---
 lib/autobuild/packages/autotools.rb |  8 --------
 lib/autobuild/packages/cmake.rb     |  3 ---
 lib/autobuild/packages/genom.rb     |  4 ----
 lib/autobuild/packages/gnumake.rb   |  2 --
 lib/autobuild/packages/import.rb    |  3 ---
 lib/autobuild/packages/pkgconfig.rb |  2 --
 lib/autobuild/packages/python.rb    |  3 ---
 lib/autobuild/progress_display.rb   |  3 ---
 lib/autobuild/reporting.rb          |  3 ---
 18 files changed, 20 insertions(+), 68 deletions(-)

diff --git a/lib/autobuild.rb b/lib/autobuild.rb
index 98d7b535..1fe5bc6d 100644
--- a/lib/autobuild.rb
+++ b/lib/autobuild.rb
@@ -11,7 +11,6 @@ module Autobuild
     extend Logger::Root('Autobuild', Logger::INFO)
 end
 
-require 'net/smtp'
 require 'socket'
 require 'etc'
 require 'find'
@@ -21,8 +20,27 @@ module Autobuild
 require 'fileutils'
 require 'optparse'
 require 'singleton'
+require 'open3'
+require 'English'
 require 'pastel'
+require 'fcntl'
+require 'rexml'
 require 'tty-prompt'
+require 'time'
+require 'set'
+require 'rbconfig'
+require 'digest/sha1'
+require 'open-uri'
+require 'net/http'
+require 'net/https'
+require 'net/smtp'
+require 'rubygems/version'
+
+require "concurrent/atomic/atomic_boolean"
+require "concurrent/array"
+
+require 'utilrb/hash/map_value'
+require 'utilrb/kernel/options'
 require 'autobuild/tools'
 
 require 'autobuild/version'
@@ -50,6 +68,7 @@ module Autobuild
 require 'autobuild/package'
 require 'autobuild/configurable'
 require 'autobuild/packages/autotools'
+require 'autobuild/packages/gnumake'
 require 'autobuild/packages/cmake'
 require 'autobuild/packages/genom'
 require 'autobuild/packages/import'
diff --git a/lib/autobuild/build_logfile.rb b/lib/autobuild/build_logfile.rb
index 037facd6..267acb88 100644
--- a/lib/autobuild/build_logfile.rb
+++ b/lib/autobuild/build_logfile.rb
@@ -1,5 +1,3 @@
-require 'time'
-
 module Autobuild
     # Parse and manipulate the information stored in a build log file (usually
     # in prefix/log/stats.log)
diff --git a/lib/autobuild/environment.rb b/lib/autobuild/environment.rb
index c42d1b84..ac9e0dca 100644
--- a/lib/autobuild/environment.rb
+++ b/lib/autobuild/environment.rb
@@ -1,7 +1,3 @@
-require 'set'
-require 'rbconfig'
-require 'utilrb/hash/map_value'
-
 module Autobuild
     @windows = RbConfig::CONFIG["host_os"] =~ /(msdos|mswin|djgpp|mingw|[Ww]indows)/
     def self.windows?
diff --git a/lib/autobuild/import/archive.rb b/lib/autobuild/import/archive.rb
index c7f842c4..eb9331a9 100644
--- a/lib/autobuild/import/archive.rb
+++ b/lib/autobuild/import/archive.rb
@@ -1,10 +1,3 @@
-require 'autobuild/importer'
-require 'digest/sha1'
-require 'open-uri'
-require 'fileutils'
-require 'net/http'
-require 'net/https'
-
 module Autobuild
     class ArchiveImporter < Importer
         # rubocop:disable Naming/ConstantName
diff --git a/lib/autobuild/import/darcs.rb b/lib/autobuild/import/darcs.rb
index 5faae12c..f1f19e46 100644
--- a/lib/autobuild/import/darcs.rb
+++ b/lib/autobuild/import/darcs.rb
@@ -1,7 +1,3 @@
-require 'autobuild/config'
-require 'autobuild/subcommand'
-require 'autobuild/importer'
-
 module Autobuild
     class DarcsImporter < Importer
         # Creates a new importer which gets the source from the Darcs
diff --git a/lib/autobuild/import/git.rb b/lib/autobuild/import/git.rb
index aa99490b..c8a9131c 100644
--- a/lib/autobuild/import/git.rb
+++ b/lib/autobuild/import/git.rb
@@ -1,10 +1,3 @@
-require 'fileutils'
-require 'autobuild/subcommand'
-require 'autobuild/importer'
-require 'utilrb/kernel/options'
-require 'open3'
-require 'English'
-
 module Autobuild
     class Git < Importer
         # Exception raised when a network access is needed while only_local is true
diff --git a/lib/autobuild/import/hg.rb b/lib/autobuild/import/hg.rb
index f020189d..bfa66bfd 100644
--- a/lib/autobuild/import/hg.rb
+++ b/lib/autobuild/import/hg.rb
@@ -1,8 +1,3 @@
-require 'fileutils'
-require 'autobuild/subcommand'
-require 'autobuild/importer'
-require 'utilrb/kernel/options'
-
 module Autobuild
     class Hg < Importer
         # Creates an importer which tracks the given repository
diff --git a/lib/autobuild/import/svn.rb b/lib/autobuild/import/svn.rb
index 85941ff0..aab6195b 100644
--- a/lib/autobuild/import/svn.rb
+++ b/lib/autobuild/import/svn.rb
@@ -1,7 +1,3 @@
-require 'autobuild/subcommand'
-require 'autobuild/importer'
-require 'rexml/document'
-
 module Autobuild
     class SVN < Importer
         # Creates an importer which gets the source for the Subversion URL +source+.
diff --git a/lib/autobuild/importer.rb b/lib/autobuild/importer.rb
index 37a53dd4..a566b048 100644
--- a/lib/autobuild/importer.rb
+++ b/lib/autobuild/importer.rb
@@ -1,6 +1,3 @@
-require 'autobuild/config'
-require 'autobuild/exceptions'
-
 # This class is the base class for objects that are used to get the source from
 # various RCS into the package source directory. A list of patches to apply
 # after the import can be given in the +:patches+ option.
diff --git a/lib/autobuild/packages/autotools.rb b/lib/autobuild/packages/autotools.rb
index ab6baadd..f9808bdb 100644
--- a/lib/autobuild/packages/autotools.rb
+++ b/lib/autobuild/packages/autotools.rb
@@ -1,11 +1,3 @@
-require 'pathname'
-require 'autobuild/timestamps'
-require 'autobuild/environment'
-require 'autobuild/package'
-require 'autobuild/subcommand'
-require 'shellwords'
-require 'fileutils'
-
 module Autobuild
     def self.autotools(opts, &proc)
         Autotools.new(opts, &proc)
diff --git a/lib/autobuild/packages/cmake.rb b/lib/autobuild/packages/cmake.rb
index 82b8e04c..ca5df609 100644
--- a/lib/autobuild/packages/cmake.rb
+++ b/lib/autobuild/packages/cmake.rb
@@ -1,6 +1,3 @@
-require 'autobuild/configurable'
-require 'autobuild/packages/gnumake'
-
 module Autobuild
     def self.cmake(options, &block)
         CMake.new(options, &block)
diff --git a/lib/autobuild/packages/genom.rb b/lib/autobuild/packages/genom.rb
index 69927a2f..d03b9699 100644
--- a/lib/autobuild/packages/genom.rb
+++ b/lib/autobuild/packages/genom.rb
@@ -1,7 +1,3 @@
-require 'autobuild/packages/autotools'
-require 'open3'
-require 'autobuild/pkgconfig'
-
 module Autobuild
     def self.genom(opts, &proc)
         GenomModule.new(opts, &proc)
diff --git a/lib/autobuild/packages/gnumake.rb b/lib/autobuild/packages/gnumake.rb
index 2aa45032..d088ecd7 100644
--- a/lib/autobuild/packages/gnumake.rb
+++ b/lib/autobuild/packages/gnumake.rb
@@ -1,5 +1,3 @@
-require 'rubygems/version'
-
 module Autobuild
     def self.reset_gnumake_detection
         @make_is_gnumake = Hash.new
diff --git a/lib/autobuild/packages/import.rb b/lib/autobuild/packages/import.rb
index 20444b55..de5dfdc8 100644
--- a/lib/autobuild/packages/import.rb
+++ b/lib/autobuild/packages/import.rb
@@ -1,6 +1,3 @@
-require 'autobuild/timestamps'
-require 'autobuild/package'
-
 module Autobuild
     def self.import(spec, &proc)
         ImporterPackage.new(spec, &proc)
diff --git a/lib/autobuild/packages/pkgconfig.rb b/lib/autobuild/packages/pkgconfig.rb
index 75e4ca63..3c9e8da7 100644
--- a/lib/autobuild/packages/pkgconfig.rb
+++ b/lib/autobuild/packages/pkgconfig.rb
@@ -1,5 +1,3 @@
-require 'autobuild/pkgconfig'
-
 module Autobuild
     class InstalledPkgConfig < Package
         attr_reader :pkgconfig, :prefix
diff --git a/lib/autobuild/packages/python.rb b/lib/autobuild/packages/python.rb
index 9230287f..0832fe20 100644
--- a/lib/autobuild/packages/python.rb
+++ b/lib/autobuild/packages/python.rb
@@ -1,6 +1,3 @@
-require 'autobuild/configurable'
-require 'open3'
-
 # Main Autobuild module
 module Autobuild
     def self.python(opts, &proc)
diff --git a/lib/autobuild/progress_display.rb b/lib/autobuild/progress_display.rb
index 3bf909d4..ae4e0d2d 100644
--- a/lib/autobuild/progress_display.rb
+++ b/lib/autobuild/progress_display.rb
@@ -1,6 +1,3 @@
-require "concurrent/atomic/atomic_boolean"
-require "concurrent/array"
-
 module Autobuild
     # Management of the progress display
     class ProgressDisplay
diff --git a/lib/autobuild/reporting.rb b/lib/autobuild/reporting.rb
index bc95bfe0..cb1d2189 100644
--- a/lib/autobuild/reporting.rb
+++ b/lib/autobuild/reporting.rb
@@ -1,6 +1,3 @@
-require 'autobuild/exceptions'
-require 'pastel'
-
 module Autobuild
     @colorizer = Pastel.new
     class << self

From 9dd7296fbf06d9b353e904e5bf79e844f88920f8 Mon Sep 17 00:00:00 2001
From: Sylvain Joyeux <sylvain.joyeux@tidewise.io>
Date: Wed, 30 Apr 2025 13:40:18 -0300
Subject: [PATCH 2/8] add Gemfile.lock to .gitignore

---
 .gitignore | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index c5f13aca..9c74064e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -5,4 +5,5 @@ vendor/
 .bundle
 coverage/
 
-.yardoc/
\ No newline at end of file
+.yardoc/
+Gemfile.lock

From 9286287416e86106980de17846a1d3fa0d0be3f1 Mon Sep 17 00:00:00 2001
From: Sylvain Joyeux <sylvain.joyeux@tidewise.io>
Date: Wed, 30 Apr 2025 13:38:41 -0300
Subject: [PATCH 3/8] chore: cleanup the subprocess implementation

- split the code in smaller methods
- rename the file into subprocess.rb since the class is called Subprocess
---
 lib/autobuild.rb                              |   2 +-
 .../{subcommand.rb => subprocess.rb}          | 398 ++++++++++--------
 test/test_subcommand.rb                       |  41 --
 test/test_subprocess.rb                       | 204 +++++++++
 4 files changed, 434 insertions(+), 211 deletions(-)
 rename lib/autobuild/{subcommand.rb => subprocess.rb} (61%)
 delete mode 100644 test/test_subcommand.rb
 create mode 100644 test/test_subprocess.rb

diff --git a/lib/autobuild.rb b/lib/autobuild.rb
index 1fe5bc6d..4aab3afb 100644
--- a/lib/autobuild.rb
+++ b/lib/autobuild.rb
@@ -49,7 +49,7 @@ module Autobuild
 require 'autobuild/pkgconfig'
 require 'autobuild/reporting'
 require 'autobuild/mail_reporter'
-require 'autobuild/subcommand'
+require 'autobuild/subprocess'
 require 'autobuild/timestamps'
 require 'autobuild/parallel'
 require 'autobuild/utility'
diff --git a/lib/autobuild/subcommand.rb b/lib/autobuild/subprocess.rb
similarity index 61%
rename from lib/autobuild/subcommand.rb
rename to lib/autobuild/subprocess.rb
index 003349e1..f49d1463 100644
--- a/lib/autobuild/subcommand.rb
+++ b/lib/autobuild/subprocess.rb
@@ -1,9 +1,3 @@
-require 'set'
-require 'autobuild/exceptions'
-require 'autobuild/reporting'
-require 'fcntl'
-require 'English'
-
 module Autobuild
     @logfiles = Set.new
     def self.clear_logfiles
@@ -212,7 +206,7 @@ def self.transparent_mode=(flag)
     # @option options [String] :input the path to a file whose content should be
     #   fed to the command standard input
     # @return [String] the command standard output
-    def self.run(target, phase, *command)
+    def self.run(target, phase, *command, &output_filter)
         STDOUT.sync = true
 
         input_streams = []
@@ -242,13 +236,8 @@ def self.run(target, phase, *command)
         command.reject! { |o| o.nil? || (o.respond_to?(:empty?) && o.empty?) }
         command.collect!(&:to_s)
 
-        if target.respond_to?(:name)
-            target_name = target.name
-            target_type = target.class
-        else
-            target_name = target.to_str
-            target_type = nil
-        end
+        target_name, target_type = target_argument_to_name_and_type(target)
+
         logdir = if target.respond_to?(:logdir)
                      target.logdir
                  else
@@ -259,27 +248,6 @@ def self.run(target, phase, *command)
             options[:working_directory] ||= target.working_directory
         end
 
-        logname = File.join(logdir, "#{target_name.gsub(/:/, '_')}-"\
-            "#{phase.to_s.gsub(/:/, '_')}.log")
-        unless File.directory?(File.dirname(logname))
-            FileUtils.mkdir_p File.dirname(logname)
-        end
-
-        if Autobuild.verbose
-            Autobuild.message "#{target_name}: running #{command.join(' ')}\n"\
-                "    (output goes to #{logname})"
-        end
-
-        open_flag = if Autobuild.keep_oldlogs then 'a'
-                    elsif Autobuild.registered_logfile?(logname) then 'a'
-                    else
-                        'w'
-                    end
-        open_flag << ":BINARY"
-
-        Autobuild.register_logfile(logname)
-        subcommand_output = Array.new
-
         env = options[:env].dup
         if options[:env_inherit]
             ENV.each do |k, v|
@@ -287,169 +255,117 @@ def self.run(target, phase, *command)
             end
         end
 
-        status = File.open(logname, open_flag) do |logfile|
-            logfile.puts if Autobuild.keep_oldlogs
-            logfile.puts
-            logfile.puts "#{Time.now}: running"
-            logfile.puts "    #{command.join(' ')}"
-            logfile.puts "with environment:"
-            env.keys.sort.each do |key|
-                if (value = env[key])
-                    logfile.puts "  '#{key}'='#{value}'"
-                end
-            end
-            logfile.puts
-            logfile.puts "#{Time.now}: running"
-            logfile.puts "    #{command.join(' ')}"
-            logfile.flush
-            logfile.sync = true
+        if Autobuild.windows?
+            windows_support(options, command)
+            return
+        end
 
-            unless input_streams.empty?
-                pread, pwrite = IO.pipe # to feed subprocess stdin
-            end
+        logname = compute_log_path(target_name, phase, logdir)
 
-            outread, outwrite = IO.pipe
-            outread.sync = true
-            outwrite.sync = true
+        status, subcommand_output = open_logfile(logname) do |logfile|
+            logfile_header(logfile, command, env)
 
-            cread, cwrite = IO.pipe # to control that exec goes well
+            if Autobuild.verbose
+                Autobuild.message "#{target_name}: running #{command.join(' ')}\n"\
+                    "    (output goes to #{logname})"
+            end
 
-            if Autobuild.windows?
-                Dir.chdir(options[:working_directory]) do
-                    unless system(*command)
-                        exit_code = $CHILD_STATUS.exitstatus
-                        raise Failed.new(exit_code, nil),
-                              "'#{command.join(' ')}' returned status #{exit_code}"
-                    end
-                end
-                return # rubocop:disable Lint/NonLocalExitFromIterator
+            unless input_streams.empty?
+                stdin_r, stdin_w = IO.pipe # to feed subprocess stdin
             end
 
-            cwrite.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
+            out_r, out_w = IO.pipe
+            out_r.sync = true
+            out_w.sync = true
+
+            control_r, control_w = IO.pipe # to control that exec goes well
+            control_w.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
 
             pid = fork do
-                    logfile.puts "in directory #{options[:working_directory] || Dir.pwd}"
+                logfile.puts "in directory #{options[:working_directory] || Dir.pwd}"
 
-                    cwrite.sync = true
-                    if Autobuild.nice
-                        Process.setpriority(Process::PRIO_PROCESS, 0, Autobuild.nice)
-                    end
+                control_r.close
+                out_r.close
+                stdin_w&.close
 
-                    outread.close
-                    $stderr.reopen(outwrite.dup)
-                    $stdout.reopen(outwrite.dup)
+                control_w.sync = true
+                $stderr.reopen(out_w.dup)
+                $stdout.reopen(out_w.dup)
+                $stdin.reopen(stdin_r) if stdin_r
 
-                    unless input_streams.empty?
-                        pwrite.close
-                        $stdin.reopen(pread)
-                    end
+                if Autobuild.nice
+                    Process.setpriority(Process::PRIO_PROCESS, 0, Autobuild.nice)
+                end
 
-                    exec(env, *command,
-                         chdir: options[:working_directory] || Dir.pwd,
-                         close_others: false)
+                exec(env, *command,
+                     chdir: options[:working_directory] || Dir.pwd,
+                     close_others: false)
             rescue Errno::ENOENT
-                    cwrite.write([CONTROL_COMMAND_NOT_FOUND].pack('I'))
-                    exit(100)
+                control_w.write([CONTROL_COMMAND_NOT_FOUND].pack('I'))
+                exit(100)
             rescue Interrupt
-                    cwrite.write([CONTROL_INTERRUPT].pack('I'))
-                    exit(100)
+                control_w.write([CONTROL_INTERRUPT].pack('I'))
+                exit(100)
             rescue ::Exception => e
-                    STDERR.puts e
-                    STDERR.puts e.backtrace.join("\n  ")
-                    cwrite.write([CONTROL_UNEXPECTED].pack('I'))
-                    exit(100)
+                STDERR.puts e
+                STDERR.puts e.backtrace.join("\n  ")
+                control_w.write([CONTROL_UNEXPECTED].pack('I'))
+                exit(100)
             end
 
-            readbuffer = StringIO.new
-
             # Feed the input
             unless input_streams.empty?
-                pread.close
-                begin
-                    input_streams.each do |instream|
-                        instream.each_line do |line|
-                            while IO.select([outread], nil, nil, 0)
-                                readbuffer.write(outread.readpartial(128))
-                            end
-                            pwrite.write(line)
-                        end
-                    end
-                rescue Errno::ENOENT => e
-                    raise Failed.new(nil, false),
-                          "cannot open input files: #{e.message}", retry: false
-                end
-                pwrite.close
+                stdin_r.close
+                readbuffer = feed_input(input_streams, out_r, stdin_w)
+                stdin_w.close
             end
 
             # Get control status
-            cwrite.close
-            value = cread.read(4)
-            if value
-                # An error occured
-                value = value.unpack1('I')
-                case value
-                when CONTROL_COMMAND_NOT_FOUND
-                    raise Failed.new(nil, false), "command '#{command.first}' not found"
-                when CONTROL_INTERRUPT
-                    raise Interrupt, "command '#{command.first}': interrupted by user"
-                else
-                    raise Failed.new(nil, false), "something unexpected happened"
-                end
-            end
-
-            transparent_prefix = "#{target_name}:#{phase}: "
-            transparent_prefix = "#{target_type}:#{transparent_prefix}" if target_type
+            control_w.close
+            process_exec_status(control_r, command)
 
             # If the caller asked for process output, provide it to him
             # line-by-line.
-            outwrite.close
+            out_w.close
 
             unless input_streams.empty?
-                readbuffer.write(outread.read)
+                readbuffer.write(out_r.read)
                 readbuffer.seek(0)
-                outread.close
-                outread = readbuffer
+                out_r.close
+                out_r = readbuffer
             end
 
-            outread.each_line do |line|
-                line.force_encoding(options[:encoding])
-                line = line.chomp
-                subcommand_output << line
-
-                logfile.puts line
-
-                if Autobuild.verbose || transparent_mode?
-                    STDOUT.puts "#{transparent_prefix}#{line}"
-                elsif block_given?
-                    # Do not yield
-                    # would mix the progress output with the actual command
-                    # output. Assume that if the user wants the command output,
-                    # the autobuild progress output is unnecessary
-                    yield(line)
-                end
-            end
-            outread.close
+            transparent_prefix =
+                transparent_output_prefix(target_name, phase, target_type)
+            subcommand_output = process_output(
+                out_r, logfile, transparent_prefix, options[:encoding], &output_filter
+            )
+            out_r.close
 
             _, childstatus = Process.wait2(pid)
             logfile.puts "Exit: #{childstatus}"
-            childstatus
+            [childstatus, subcommand_output]
         end
 
-        if !status.exitstatus || status.exitstatus > 0
-            if status.termsig == 2 # SIGINT == 2
-                raise Interrupt, "subcommand #{command.join(' ')} interrupted"
-            end
+        handle_exit_status(status, command)
+        update_stats(target, phase, start_time)
 
-            if status.termsig
-                raise Failed.new(status.exitstatus, nil),
-                      "'#{command.join(' ')}' terminated by signal #{status.termsig}"
-            else
-                raise Failed.new(status.exitstatus, nil),
-                      "'#{command.join(' ')}' returned status #{status.exitstatus}"
-            end
-        end
+        subcommand_output
+    rescue Failed => e
+        error = Autobuild::SubcommandFailed.new(
+            target, command.join(" "), logname, e.status, subcommand_output || []
+        )
+        error.retry = if e.retry?.nil? then options[:retry]
+                      else
+                          e.retry?
+                      end
+        error.phase = phase
+        raise error, e.message
+    end
 
+    def self.update_stats(target, phase, start_time)
         duration = Time.now - start_time
+        target_name, = target_argument_to_name_and_type(target)
         Autobuild.add_stat(target, phase, duration)
         FileUtils.mkdir_p(Autobuild.logdir)
         File.open(File.join(Autobuild.logdir, "stats.log"), 'a') do |io|
@@ -458,15 +374,159 @@ def self.run(target, phase, *command)
             io.puts "#{formatted_time} #{target_name} #{phase} #{duration}"
         end
         target.add_stat(phase, duration) if target.respond_to?(:add_stat)
+    end
+
+    def self.target_argument_to_name_and_type(target)
+        if target.respond_to?(:name)
+            [target.name, target.class]
+        else
+            [target.to_str, nil]
+        end
+    end
+
+    def self.target_argument_to_name(target)
+        if target.respond_to?(:name)
+            target.name
+        else
+            target.to_str
+        end
+    end
+
+    def self.target_argument_to_type(target, type)
+        return type if type
+
+        target.class if target.respond_to?(:name)
+    end
+
+    def self.handle_exit_status(status, command)
+        return if status.exitstatus == 0
+
+        if status.termsig == 2 # SIGINT == 2
+            raise Interrupt, "subcommand #{command.join(' ')} interrupted"
+        end
+
+        if status.termsig
+            raise Failed.new(status.exitstatus, nil),
+                  "'#{command.join(' ')}' terminated by signal #{status.termsig}"
+        else
+            raise Failed.new(status.exitstatus, nil),
+                  "'#{command.join(' ')}' returned status #{status.exitstatus}"
+        end
+    end
+
+    def self.windows_support(options, command)
+        Dir.chdir(options[:working_directory]) do
+            unless system(*command)
+                exit_code = $CHILD_STATUS.exitstatus
+                raise Failed.new(exit_code, nil),
+                      "'#{command.join(' ')}' returned status #{exit_code}"
+            end
+        end
+    end
+
+    def self.compute_log_path(target_name, phase, logdir)
+        File.join(logdir, "#{target_name.gsub(/:/, '_')}-"\
+            "#{phase.to_s.gsub(/:/, '_')}.log")
+    end
+
+    def self.open_logfile(logname, &block)
+        open_flag = if Autobuild.keep_oldlogs then 'a'
+                    elsif Autobuild.registered_logfile?(logname) then 'a'
+                    else
+                        'w'
+                    end
+        open_flag << ":BINARY"
+
+        unless File.directory?(File.dirname(logname))
+            FileUtils.mkdir_p File.dirname(logname)
+        end
+
+        Autobuild.register_logfile(logname)
+        File.open(logname, open_flag, &block)
+    end
+
+    def self.logfile_header(logfile, command, env)
+        logfile.puts if Autobuild.keep_oldlogs
+        logfile.puts
+        logfile.puts "#{Time.now}: running"
+        logfile.puts "    #{command.join(' ')}"
+        logfile.puts "with environment:"
+        env.keys.sort.each do |key|
+            if (value = env[key])
+                logfile.puts "  '#{key}'='#{value}'"
+            end
+        end
+        logfile.puts
+        logfile.puts "#{Time.now}: running"
+        logfile.puts "    #{command.join(' ')}"
+        logfile.flush
+        logfile.sync = true
+    end
+
+    def self.process_output(
+        out_r, logfile, transparent_prefix, encoding, &filter
+    )
+        subcommand_output = []
+        out_r.each_line do |line|
+            line.force_encoding(encoding)
+            line = line.chomp
+            subcommand_output << line
+
+            logfile.puts line
+
+            if Autobuild.verbose || transparent_mode?
+                STDOUT.puts "#{transparent_prefix}#{line}"
+            elsif filter
+                # Do not yield
+                # would mix the progress output with the actual command
+                # output. Assume that if the user wants the command output,
+                # the autobuild progress output is unnecessary
+                filter.call(line)
+            end
+        end
         subcommand_output
-    rescue Failed => e
-        error = Autobuild::SubcommandFailed.new(target, command.join(" "),
-                                                logname, e.status, subcommand_output)
-        error.retry = if e.retry?.nil? then options[:retry]
-                      else
-                          e.retry?
-                      end
-        error.phase = phase
-        raise error, e.message
+    end
+
+    def self.process_exec_status(control_r, command)
+        return unless (value = control_r.read(4))
+
+        # An error occured
+        value = value.unpack1('I')
+        case value
+        when CONTROL_COMMAND_NOT_FOUND
+            raise Failed.new(nil, false), "command '#{command.first}' not found"
+        when CONTROL_INTERRUPT
+            raise Interrupt, "command '#{command.first}': interrupted by user"
+        else
+            raise Failed.new(nil, false), "something unexpected happened"
+        end
+    end
+
+    def self.transparent_output_prefix(target_name, phase, target_type)
+        prefix = "#{target_name}:#{phase}: "
+        return prefix unless target_type
+
+        "#{target_type}:#{prefix}"
+    end
+
+    def self.feed_input(input_streams, out_r, stdin_w)
+        readbuffer = StringIO.new
+        input_streams.each do |instream|
+            instream.each_line do |line|
+                # Read the process output to avoid having it block on a full pipe
+                begin
+                    loop do
+                        readbuffer.write(out_r.read_nonblock(1024))
+                    end
+                rescue IO::WaitReadable # rubocop:disable Lint/SuppressedException
+                end
+
+                stdin_w.write(line)
+            end
+        end
+        readbuffer
+    rescue Errno::ENOENT => e
+        raise Failed.new(nil, false),
+              "cannot open input files: #{e.message}", retry: false
     end
 end
diff --git a/test/test_subcommand.rb b/test/test_subcommand.rb
deleted file mode 100644
index 3f8c0b5a..00000000
--- a/test/test_subcommand.rb
+++ /dev/null
@@ -1,41 +0,0 @@
-require 'autobuild/test'
-
-class TestSubcommand < Minitest::Test
-    EXAMPLE_1 = <<-EXAMPLE_END.freeze
-This is a file
-It will be the first part of the two-part cat
-    EXAMPLE_END
-
-    EXAMPLE_2 = <<-EXAMPLE_END.freeze
-This is another file
-It will be the second part of the two-part cat
-    EXAMPLE_END
-
-    attr_reader :source1, :source2
-
-    def setup
-        super
-
-        Autobuild.logdir = tempdir
-
-        # Write example files
-        @source1 = File.join(tempdir, 'source1')
-        @source2 = File.join(tempdir, 'source2')
-        File.open(source1, 'w+') { |f| f.write(EXAMPLE_1) }
-        File.open(source2, 'w+') { |f| f.write(EXAMPLE_2) }
-    end
-
-    def test_behaviour_on_unexpected_error
-        flexmock(Autobuild::Subprocess).should_receive(:exec).and_raise(::Exception)
-        assert_raises(Autobuild::SubcommandFailed) { Autobuild::Subprocess.run('test', 'test', 'does_not_exist') }
-    end
-
-    def test_behaviour_on_inexistent_command
-        assert_raises(Autobuild::SubcommandFailed) { Autobuild::Subprocess.run('test', 'test', 'does_not_exist') }
-    end
-
-    def test_behaviour_on_interrupt
-        flexmock(Autobuild::Subprocess).should_receive(:exec).and_raise(Interrupt)
-        assert_raises(Interrupt) { Autobuild::Subprocess.run('test', 'test', 'does_not_exist') }
-    end
-end
diff --git a/test/test_subprocess.rb b/test/test_subprocess.rb
new file mode 100644
index 00000000..b16f4c13
--- /dev/null
+++ b/test/test_subprocess.rb
@@ -0,0 +1,204 @@
+require 'autobuild/test'
+
+module Autobuild
+    describe Subprocess do
+        before do
+            @example1 = <<~EXAMPLE_END.freeze
+                This is a file
+                It will be the first part of the two-part cat
+            EXAMPLE_END
+
+            @example2 = <<~EXAMPLE_END.freeze
+                This is a file
+                It will be the second part of the two-part cat
+            EXAMPLE_END
+
+            Autobuild.logdir = tempdir
+
+            # Write example files
+            @source1 = File.join(tempdir, 'source1')
+            @source2 = File.join(tempdir, 'source2')
+            File.open(@source1, 'w+') { |f| f.write(@example1) }
+            File.open(@source2, 'w+') { |f| f.write(@example2) }
+        end
+
+        after do
+            Autobuild.keep_oldlogs = true
+            Autobuild::Subprocess.transparent_mode = false
+        end
+
+        describe "basic execution behavior" do
+            it "executes a subcommand and returns the command output" do
+                result = Subprocess.run("test", "phase", "cat", @source1)
+                assert_equal @example1.chomp.split("\n"), result
+            end
+
+            it "raises SubcommandFailed if the command failed" do
+                e = assert_raises(SubcommandFailed) do
+                    Subprocess.run(
+                        "test", "phase", "cat", "/does/not/exist",
+                        env: { "LANG" => "C" }
+                    )
+                end
+
+                assert_equal "cat /does/not/exist", e.command
+                assert_equal 1, e.status
+                assert_match(/No such file or directory/, e.output.first)
+            end
+
+            it "sets the SubcommandFailed retry flag to false by default" do
+                e = assert_raises(SubcommandFailed) do
+                    Subprocess.run("test", "phase", "cat", "/does/not/exist")
+                end
+                refute e.retry?
+            end
+
+            it "sets the SubcommandFailed retry flag to true "\
+               "if the retry argument is true" do
+                e = assert_raises(SubcommandFailed) do
+                    Subprocess.run("test", "phase", "cat", "/does/not/exist", retry: true)
+                end
+                assert e.retry?
+            end
+
+            it "does not retry errors to spawn the command" do
+                e = assert_raises(SubcommandFailed) do
+                    Subprocess.run("test", "phase", "/does/not/exist", retry: true)
+                end
+                refute e.retry?
+            end
+
+            it "passes the given environment" do
+                result = Subprocess.run(
+                    "test", "phase", "sh", "-c", "echo $FOO", env: { "FOO" => "TEST" }
+                )
+                assert_equal ["TEST"], result
+            end
+
+            it "executes the command in the provided working directory, if given" do
+                dir = make_tmpdir
+                result = Subprocess.run(
+                    "test", "phase", "pwd",
+                    working_directory: dir
+                )
+                assert_equal [dir], result
+            end
+
+            it "yields the command output if a block is given" do
+                lines = []
+                result = Subprocess.run("test", "phase", "cat", @source1) do |line|
+                    lines << line
+                end
+                assert_equal @example1.chomp.split("\n"), lines
+                assert_equal @example1.chomp.split("\n"), result
+            end
+
+            it "passes the command output through if transparent mode is set" do
+                Autobuild::Subprocess.transparent_mode = true
+                stdout = []
+                block = []
+                flexmock(STDOUT).should_receive(:puts).and_return { |line| stdout << line }
+                result = Subprocess.run("test", "phase", "cat", @source1) do |line|
+                    block << line
+                end
+
+                expected_lines = @example1.chomp.split("\n")
+                assert_equal expected_lines, result
+                with_prefix = expected_lines.map { |l| "test:phase: #{l}" }
+                assert_equal with_prefix, stdout
+                assert_equal [], block
+            end
+
+            it "reports that a command was terminated by a signal "\
+               "from within the error message" do
+                e = assert_raises(SubcommandFailed) do
+                    Subprocess.run("test", "phase", "sh", "-c", "kill -KILL $$")
+                end
+                assert_equal "'sh -c kill -KILL $$' terminated by signal 9",
+                             e.message.split("\n")[-2].strip
+            end
+
+            it "raises Interrupt if the command was killed with SIGINT" do
+                assert_raises(Interrupt) do
+                    Subprocess.run("test", "phase", "sh", "-c", "kill -INT $$")
+                end
+            end
+        end
+
+        describe "input handling" do
+            it "passes data from the file in 'input' to the subprocess" do
+                result = Autobuild::Subprocess.run(
+                    "test", "phase", "cat", input: @source1
+                )
+                assert_equal @example1.chomp.split("\n"), result
+            end
+
+            it "passes I/O input from input_streams to the subprocess" do
+                result = File.open(@source1) do |source1_io|
+                    File.open(@source2) do |source2_io|
+                        Autobuild::Subprocess.run(
+                            "test", "phase", "cat",
+                            input_streams: [source1_io, source2_io]
+                        )
+                    end
+                end
+
+                expected =
+                    @example1.chomp.split("\n") +
+                    @example2.chomp.split("\n")
+                assert_equal expected, result
+            end
+        end
+
+        describe "log file management" do
+            it "saves the subcommand output to a log file" do
+                Subprocess.run("test", "phase", "cat", @source1)
+                actual = File.read(File.join(Autobuild.logdir, "test-phase.log"))
+                actual_lines = actual.split("\n")
+
+                expected_lines = @example1.chomp.split("\n")
+                assert_match Regexp.new(Regexp.quote("cat #{@source1}")), actual
+                assert_equal expected_lines, actual_lines[-(expected_lines.size + 1)..-2]
+            end
+
+            it "handles package names with slashes" do
+                Subprocess.run("dir/test", "phase", "cat", @source1)
+                actual = File.read(File.join(Autobuild.logdir, "dir", "test-phase.log"))
+                actual_lines = actual.split("\n")
+
+                expected_lines = @example1.chomp.split("\n")
+                assert_match Regexp.new(Regexp.quote("cat #{@source1}")), actual
+                assert_equal expected_lines, actual_lines[-(expected_lines.size + 1)..-2]
+            end
+
+            it "appends to an old logfile if Autobuild.keep_oldlogs is set" do
+                Autobuild.keep_oldlogs = true
+                File.write(File.join(Autobuild.logdir, "test-phase.log"), "old content")
+                Subprocess.run("test", "phase", "cat", @source1)
+
+                actual = File.read(File.join(Autobuild.logdir, "test-phase.log"))
+                assert_equal "old content\n", actual.each_line.first
+                assert_match Regexp.new(Regexp.quote("cat #{@source1}")), actual
+            end
+
+            it "overwrites an old logfile if Autobuild.keep_oldlogs is unset" do
+                Autobuild.keep_oldlogs = false
+                File.write(File.join(Autobuild.logdir, "test-phase.log"), "old content")
+                Subprocess.run("test", "phase", "cat", @source1)
+
+                actual = File.read(File.join(Autobuild.logdir, "test-phase.log"))
+                refute_equal "old content\n", actual.each_line.first
+                assert_match Regexp.new(Regexp.quote("cat #{@source1}")), actual
+            end
+
+            it "always keeps a logfile that was produced in the same run" do
+                Autobuild.keep_oldlogs = false
+                Subprocess.run("test", "phase", "cat", @source1)
+                Subprocess.run("test", "phase", "cat", @source2)
+                actual = File.read(File.join(Autobuild.logdir, "test-phase.log"))
+                assert_match Regexp.new(Regexp.quote("cat #{@source1}")), actual
+                assert_match Regexp.new(Regexp.quote("cat #{@source2}")), actual
+            end
+        end
+    end
+end

From cb7cff97e4efa1be846f6b2da2dfe5483bf616e2 Mon Sep 17 00:00:00 2001
From: Sylvain Joyeux <sylvain.joyeux@tidewise.io>
Date: Sun, 3 Aug 2025 16:31:10 -0300
Subject: [PATCH 4/8] fix: add missing require

---
 lib/autobuild/reporting.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/autobuild/reporting.rb b/lib/autobuild/reporting.rb
index cb1d2189..984f5eab 100644
--- a/lib/autobuild/reporting.rb
+++ b/lib/autobuild/reporting.rb
@@ -1,3 +1,7 @@
+require "pastel"
+require "concurrent/atomic/atomic_boolean"
+require "concurrent/array"
+
 module Autobuild
     @colorizer = Pastel.new
     class << self

From c967cc4abb378fe12885f49c498c9d8b3bec9a2e Mon Sep 17 00:00:00 2001
From: Sylvain Joyeux <sylvain.joyeux@tidewise.io>
Date: Sun, 3 Aug 2025 16:42:47 -0300
Subject: [PATCH 5/8] chore: add more logging to Subcommand for debugging

---
 lib/autobuild/subprocess.rb | 2 ++
 test/test_subprocess.rb     | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/autobuild/subprocess.rb b/lib/autobuild/subprocess.rb
index f49d1463..98c94782 100644
--- a/lib/autobuild/subprocess.rb
+++ b/lib/autobuild/subprocess.rb
@@ -337,11 +337,13 @@ def self.run(target, phase, *command, &output_filter)
 
             transparent_prefix =
                 transparent_output_prefix(target_name, phase, target_type)
+            logfile.puts "Processing command output"
             subcommand_output = process_output(
                 out_r, logfile, transparent_prefix, options[:encoding], &output_filter
             )
             out_r.close
 
+            logfile.puts "Waiting for #{pid} to finish"
             _, childstatus = Process.wait2(pid)
             logfile.puts "Exit: #{childstatus}"
             [childstatus, subcommand_output]
diff --git a/test/test_subprocess.rb b/test/test_subprocess.rb
index b16f4c13..8c1986c1 100644
--- a/test/test_subprocess.rb
+++ b/test/test_subprocess.rb
@@ -158,7 +158,7 @@ module Autobuild
 
                 expected_lines = @example1.chomp.split("\n")
                 assert_match Regexp.new(Regexp.quote("cat #{@source1}")), actual
-                assert_equal expected_lines, actual_lines[-(expected_lines.size + 1)..-2]
+                assert_equal expected_lines, actual_lines[-(expected_lines.size + 2)..-3]
             end
 
             it "handles package names with slashes" do
@@ -168,7 +168,7 @@ module Autobuild
 
                 expected_lines = @example1.chomp.split("\n")
                 assert_match Regexp.new(Regexp.quote("cat #{@source1}")), actual
-                assert_equal expected_lines, actual_lines[-(expected_lines.size + 1)..-2]
+                assert_equal expected_lines, actual_lines[-(expected_lines.size + 2)..-3]
             end
 
             it "appends to an old logfile if Autobuild.keep_oldlogs is set" do

From 03dd8f3bb928d5b4f3078f2efa39ff4d055bfb2f Mon Sep 17 00:00:00 2001
From: Sylvain Joyeux <sylvain.joyeux@tidewise.io>
Date: Sun, 3 Aug 2025 16:52:09 -0300
Subject: [PATCH 6/8] fix: rewrite the process output method to use readpartial

---
 lib/autobuild/subprocess.rb | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/autobuild/subprocess.rb b/lib/autobuild/subprocess.rb
index 98c94782..08571528 100644
--- a/lib/autobuild/subprocess.rb
+++ b/lib/autobuild/subprocess.rb
@@ -465,11 +465,29 @@ def self.logfile_header(logfile, command, env)
         logfile.sync = true
     end
 
+    def self.outpipe_each_line(out_r)
+        buffer = +""
+        while (data = out_r.readpartial(1024))
+            buffer << data
+            scanner = StringScanner.new(buffer)
+            while (line = scanner.scan_until(/\n/))
+                yield line
+            end
+            buffer = scanner.rest.dup
+        end
+    rescue EOFError
+        scanner = StringScanner.new(buffer)
+        while (line = scanner.scan_until(/\n/))
+            yield line
+        end
+        yield scanner.rest unless scanner.rest.empty?
+    end
+
     def self.process_output(
         out_r, logfile, transparent_prefix, encoding, &filter
     )
         subcommand_output = []
-        out_r.each_line do |line|
+        outpipe_each_line(out_r) do |line|
             line.force_encoding(encoding)
             line = line.chomp
             subcommand_output << line

From c3ee9a58bb7b001a8406ade14c474a5dc67be249 Mon Sep 17 00:00:00 2001
From: Sylvain Joyeux <sylvain.joyeux@tidewise.io>
Date: Sun, 3 Aug 2025 17:15:17 -0300
Subject: [PATCH 7/8] chore: second attempt at using spawn instead of fork

---
 lib/autobuild/subprocess.rb | 73 +++++++++----------------------------
 1 file changed, 18 insertions(+), 55 deletions(-)

diff --git a/lib/autobuild/subprocess.rb b/lib/autobuild/subprocess.rb
index 08571528..7b546ff6 100644
--- a/lib/autobuild/subprocess.rb
+++ b/lib/autobuild/subprocess.rb
@@ -159,10 +159,6 @@ def initialize(status, do_retry)
         end
     end
 
-    CONTROL_COMMAND_NOT_FOUND = 1
-    CONTROL_UNEXPECTED = 2
-    CONTROL_INTERRUPT = 3
-
     @transparent_mode = false
 
     def self.transparent_mode?
@@ -278,52 +274,34 @@ def self.run(target, phase, *command, &output_filter)
             out_r.sync = true
             out_w.sync = true
 
-            control_r, control_w = IO.pipe # to control that exec goes well
-            control_w.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
-
-            pid = fork do
-                logfile.puts "in directory #{options[:working_directory] || Dir.pwd}"
-
-                control_r.close
-                out_r.close
-                stdin_w&.close
-
-                control_w.sync = true
-                $stderr.reopen(out_w.dup)
-                $stdout.reopen(out_w.dup)
-                $stdin.reopen(stdin_r) if stdin_r
-
-                if Autobuild.nice
-                    Process.setpriority(Process::PRIO_PROCESS, 0, Autobuild.nice)
-                end
-
-                exec(env, *command,
-                     chdir: options[:working_directory] || Dir.pwd,
-                     close_others: false)
+            logfile.puts "Spawning"
+            stdin_redir = { :in => stdin_r } if stdin_r
+            begin
+                pid = spawn(
+                    env, *command,
+                    {
+                        :chdir => options[:working_directory] || Dir.pwd,
+                        :close_others => false,
+                        %I[err out] => out_w
+                    }.merge(stdin_redir || {})
+                )
+                logfile.puts "Spawned, PID=#{pid}"
             rescue Errno::ENOENT
-                control_w.write([CONTROL_COMMAND_NOT_FOUND].pack('I'))
-                exit(100)
-            rescue Interrupt
-                control_w.write([CONTROL_INTERRUPT].pack('I'))
-                exit(100)
-            rescue ::Exception => e
-                STDERR.puts e
-                STDERR.puts e.backtrace.join("\n  ")
-                control_w.write([CONTROL_UNEXPECTED].pack('I'))
-                exit(100)
+                raise Failed.new(nil, false), "command '#{command.first}' not found"
+            end
+
+            if Autobuild.nice
+                Process.setpriority(Process::PRIO_PROCESS, pid, Autobuild.nice)
             end
 
             # Feed the input
             unless input_streams.empty?
+                logfile.puts "Feeding STDIN"
                 stdin_r.close
                 readbuffer = feed_input(input_streams, out_r, stdin_w)
                 stdin_w.close
             end
 
-            # Get control status
-            control_w.close
-            process_exec_status(control_r, command)
-
             # If the caller asked for process output, provide it to him
             # line-by-line.
             out_w.close
@@ -507,21 +485,6 @@ def self.process_output(
         subcommand_output
     end
 
-    def self.process_exec_status(control_r, command)
-        return unless (value = control_r.read(4))
-
-        # An error occured
-        value = value.unpack1('I')
-        case value
-        when CONTROL_COMMAND_NOT_FOUND
-            raise Failed.new(nil, false), "command '#{command.first}' not found"
-        when CONTROL_INTERRUPT
-            raise Interrupt, "command '#{command.first}': interrupted by user"
-        else
-            raise Failed.new(nil, false), "something unexpected happened"
-        end
-    end
-
     def self.transparent_output_prefix(target_name, phase, target_type)
         prefix = "#{target_name}:#{phase}: "
         return prefix unless target_type

From 129e4adae717bb4f14806d392fb408bb0b5c9868 Mon Sep 17 00:00:00 2001
From: Sylvain Joyeux <sylvain.joyeux@tidewise.io>
Date: Fri, 8 Aug 2025 10:04:03 -0300
Subject: [PATCH 8/8] chore: add 3.2 and 3.1 to the test matrix

---
 .github/workflows/test.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 3d07f911..a8dd5a80 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -9,6 +9,8 @@ jobs:
     strategy:
       matrix:
         ruby-version:
+          - "3.2"
+          - "3.1"
           - "3.0"
           - "2.7"
           - "2.5"