From 8c64813b28e2aba06e4f32748819ef5edd3cadf0 Mon Sep 17 00:00:00 2001 From: Sylvain Date: Mon, 1 Nov 2021 17:39:23 -0300 Subject: [PATCH 1/3] fix: rewrite Subcommand.run using Kernel.spawn The current implementation was doing fork/exec by hand because it had been created under ruby 1.8, which did not have spawn. It makes zero sense nowadays, where there exists a multiplatform implementation in the language itself. --- lib/autobuild/subcommand.rb | 232 ++++++++++++++++-------------------- lib/autobuild/test.rb | 1 + test/test_subcommand.rb | 41 ------- test/test_subprocess.rb | 204 +++++++++++++++++++++++++++++++ 4 files changed, 305 insertions(+), 173 deletions(-) delete mode 100644 test/test_subcommand.rb create mode 100644 test/test_subprocess.rb diff --git a/lib/autobuild/subcommand.rb b/lib/autobuild/subcommand.rb index 69099a4b..b8da93e8 100644 --- a/lib/autobuild/subcommand.rb +++ b/lib/autobuild/subcommand.rb @@ -30,14 +30,31 @@ def self.reset_statistics @statistics = Hash.new end - def self.add_stat(package, phase, duration) - if !@statistics[package] - @statistics[package] = { phase => duration } - elsif !@statistics[package][phase] - @statistics[package][phase] = duration + def self.add_stat(target, phase, start_time) + duration = Time.now - start_time + + if !@statistics[target] + @statistics[target] = { phase => duration } + elsif !@statistics[target][phase] + @statistics[target][phase] = duration else - @statistics[package][phase] += duration + @statistics[target][phase] += duration end + + target_name = + if target.respond_to?(:name) + target.name + else + target.to_str + end + + FileUtils.mkdir_p(Autobuild.logdir) + File.open(File.join(Autobuild.logdir, "stats.log"), 'a') do |io| + formatted_msec = format('%.03i', start_time.tv_usec / 1000) + formatted_time = "#{start_time.strftime('%F %H:%M:%S')}.#{formatted_msec}" + io.puts "#{formatted_time} #{target_name} #{phase} #{duration}" + end + target.add_stat(phase, duration) if target.respond_to?(:add_stat) end reset_statistics @@ -286,129 +303,63 @@ def self.run(target, phase, *command) 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 - - unless input_streams.empty? - pread, pwrite = IO.pipe # to feed subprocess stdin + if input_streams.empty? + inread = :close + else + inread, inwrite = IO.pipe # to feed subprocess stdin end outread, outwrite = IO.pipe outread.sync = true outwrite.sync = true - cread, cwrite = IO.pipe # to control that exec goes well + chdir = options[:working_directory] || Dir.pwd + logfile_output_command_preamble(logfile, command, env, chdir) + logfile.sync = true - 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 + begin + pid = spawn( + env, *command, + chdir: chdir, in: inread, out: outwrite, err: outwrite + ) + rescue StandardError => e + raise Failed.new(nil, false), e.message end - cwrite.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC) - - pid = fork do - logfile.puts "in directory #{options[:working_directory] || Dir.pwd}" - - cwrite.sync = true - if Autobuild.nice - Process.setpriority(Process::PRIO_PROCESS, 0, Autobuild.nice) - end - - outread.close - $stderr.reopen(outwrite.dup) - $stdout.reopen(outwrite.dup) - - unless input_streams.empty? - pwrite.close - $stdin.reopen(pread) - end - - 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) - rescue Interrupt - cwrite.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) + if Autobuild.nice + Process.setpriority(Process::PRIO_PROCESS, pid, Autobuild.nice) end - readbuffer = StringIO.new + outwrite.close # 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 + # To feed multiple input streams, we have to read and write at the same + # time. Those are pipes, and buffers aren't unlimited (i.e. we could + # deadlock if we weren't doing this) + # + # To do that, we transfer the output to an internal buffer. Let's hope + # there isn't gigabytes of data ;-) + readbuffer = StringIO.new + inread.close + input_streams.each do |instream| + instream.each_line do |line| + r_ready, w_ready = IO.select([outread], [inwrite], nil, 1) + readbuffer.write(outread.readpartial(128)) unless r_ready.empty? + inwrite.write(line) unless w_ready.empty? end - rescue Errno::ENOENT => e - raise Failed.new(nil, false), - "cannot open input files: #{e.message}", retry: false end - pwrite.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 - - # If the caller asked for process output, provide it to him - # line-by-line. - outwrite.close - - unless input_streams.empty? + inwrite.close readbuffer.write(outread.read) readbuffer.seek(0) outread.close outread = readbuffer end + transparent_prefix = "#{target_name}:#{phase}: " + transparent_prefix = "#{target_type}:#{transparent_prefix}" if target_type + outread.each_line do |line| line.force_encoding(options[:encoding]) line = line.chomp @@ -418,11 +369,13 @@ def self.run(target, phase, *command) 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 + # + # This would mix the progress output with the actual command + # output. Assume that if the user wants the transparent mode + # (i.e. --tool in autoproj), the autobuild progress output + # is unnecessary + elsif block_given? yield(line) end end @@ -434,36 +387,51 @@ def self.run(target, phase, *command) end if !status.exitstatus || 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 + handle_failed_exit_status(command, status) end - duration = Time.now - start_time - Autobuild.add_stat(target, phase, duration) - FileUtils.mkdir_p(Autobuild.logdir) - File.open(File.join(Autobuild.logdir, "stats.log"), 'a') do |io| - formatted_msec = format('%.03i', start_time.tv_usec / 1000) - formatted_time = "#{start_time.strftime('%F %H:%M:%S')}.#{formatted_msec}" - io.puts "#{formatted_time} #{target_name} #{phase} #{duration}" - end - target.add_stat(phase, duration) if target.respond_to?(:add_stat) + Autobuild.add_stat(target, phase, start_time) subcommand_output rescue Failed => e - error = Autobuild::SubcommandFailed.new(target, command.join(" "), - logname, e.status, subcommand_output) + 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.logfile_output_command_preamble(io, command, env, chdir) + io.puts if Autobuild.keep_oldlogs + io.puts + io.puts "#{Time.now}: running" + io.puts " #{command.join(' ')}" + io.puts + io.puts "in directory directory #{chdir}" + io.puts + io.puts "with environment:" + env.keys.sort.each do |key| + if (value = env[key]) + io.puts " '#{key}'='#{value}'" + end + end + io.flush + end + + def self.handle_failed_exit_status(command, status) + 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 end diff --git a/lib/autobuild/test.rb b/lib/autobuild/test.rb index 8efd78d0..51ae25d3 100644 --- a/lib/autobuild/test.rb +++ b/lib/autobuild/test.rb @@ -58,6 +58,7 @@ def teardown Autobuild::Package.clear Rake::Task.clear Autobuild.reset_gnumake_detection + Autobuild.clear_logfiles @temp_dirs.each do |dir| FileUtils.rm_rf dir 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 b7138823d90d1eae48678a0e6873d4b3e3451aca Mon Sep 17 00:00:00 2001 From: Sylvain Date: Thu, 11 Nov 2021 11:02:58 -0300 Subject: [PATCH 2/3] fix: auto-correct Layout/OddElseLayout This error started appearing in a new version of rubocop. It is auto-correctable, so auto-correct it --- lib/autobuild/environment.rb | 42 +++++++++++++++++++---------- lib/autobuild/exceptions.rb | 3 ++- lib/autobuild/import/archive.rb | 3 ++- lib/autobuild/import/git.rb | 12 ++++++--- lib/autobuild/import/svn.rb | 3 ++- lib/autobuild/importer.rb | 6 +++-- lib/autobuild/package.rb | 6 +++-- lib/autobuild/packages/autotools.rb | 3 ++- lib/autobuild/packages/cmake.rb | 3 ++- lib/autobuild/parallel.rb | 3 ++- lib/autobuild/progress_display.rb | 3 ++- lib/autobuild/reporting.rb | 6 +++-- lib/autobuild/subcommand.rb | 14 +++++++--- lib/autobuild/test_utility.rb | 6 +++-- 14 files changed, 76 insertions(+), 37 deletions(-) diff --git a/lib/autobuild/environment.rb b/lib/autobuild/environment.rb index f704e159..597dae9a 100644 --- a/lib/autobuild/environment.rb +++ b/lib/autobuild/environment.rb @@ -29,37 +29,44 @@ def self.msys? SHELL_VAR_EXPANSION = if windows? then "%%%s%%".freeze - else "$%s".freeze + else + "$%s".freeze end SHELL_SET_COMMAND = if windows? then "set %s=%s".freeze - else "%s=\"%s\"".freeze + else + "%s=\"%s\"".freeze end SHELL_CONDITIONAL_SET_COMMAND = if windows? then "set %s=%s".freeze - else "if test -z \"$%1$s\"; then\n %1$s=\"%3$s\"\n"\ + else + "if test -z \"$%1$s\"; then\n %1$s=\"%3$s\"\n"\ "else\n %1$s=\"%2$s\"\nfi".freeze end SHELL_UNSET_COMMAND = "unset %s".freeze SHELL_EXPORT_COMMAND = if windows? then "set %s".freeze - else "export %s".freeze + else + "export %s".freeze end SHELL_SOURCE_SCRIPT = if windows? then "%s".freeze - else '. "%s"'.freeze + else + '. "%s"'.freeze end LIBRARY_PATH = if macos? then 'DYLD_LIBRARY_PATH'.freeze elsif windows? then 'PATH'.freeze - else 'LD_LIBRARY_PATH'.freeze + else + 'LD_LIBRARY_PATH'.freeze end LIBRARY_SUFFIX = if macos? then 'dylib' elsif windows? then 'dll' - else 'so' + else + 'so' end ORIGINAL_ENV = Hash.new @@ -211,7 +218,8 @@ def inherit?(name = nil) if @inherit if name @inherited_variables.include?(name) - else true + else + true end end end @@ -245,7 +253,8 @@ def inherit(*names) flag = if !names.last.respond_to?(:to_str) names.pop - else true + else + true end if flag @@ -342,7 +351,8 @@ def value(name, options = Hash.new) inherited_environment[name] || [] elsif inheritance_mode == :keep && inherit?(name) ["$#{name}"] - else [] + else + [] end value = [] @@ -439,7 +449,8 @@ def source_before(file = nil, shell: 'sh') if file @source_before << { file: file, shell: shell } source_before(shell: shell) # for backwards compatibility - else @source_before.select { |pair| pair[:shell] == shell } + else + @source_before.select { |pair| pair[:shell] == shell } .map { |item| item[:file] } end end @@ -457,7 +468,8 @@ def source_after(file = nil, shell: 'sh') if file @source_after << { file: file, shell: shell } source_after(shell: shell) # for backwards compatibility - else @source_after.select { |pair| pair[:shell] == shell } + else + @source_after.select { |pair| pair[:shell] == shell } .map { |item| item[:file] } end end @@ -542,7 +554,8 @@ def self.environment_from_export(export, base_env = ENV) with_inheritance = with_inheritance.map do |value| if value == variable_expansion base_env[name] - else value + else + value end end result[name] = with_inheritance.join(File::PATH_SEPARATOR) @@ -604,7 +617,8 @@ def arch_size @arch_size ||= if RbConfig::CONFIG['host_cpu'] =~ /64/ 64 - else 32 + else + 32 end @arch_size end diff --git a/lib/autobuild/exceptions.rb b/lib/autobuild/exceptions.rb index 56ca03c7..017df130 100644 --- a/lib/autobuild/exceptions.rb +++ b/lib/autobuild/exceptions.rb @@ -33,7 +33,8 @@ def to_s target_name = if target.respond_to?(:name) target.name - else target.to_str + else + target.to_str end if target && phase diff --git a/lib/autobuild/import/archive.rb b/lib/autobuild/import/archive.rb index 3c4dba02..c7f842c4 100644 --- a/lib/autobuild/import/archive.rb +++ b/lib/autobuild/import/archive.rb @@ -529,7 +529,8 @@ def checkout(package, options = Hash.new) # :nodoc: if mode == Zip main_dir = if @options[:no_subdirectory] then package.srcdir - else base_dir + else + base_dir end FileUtils.mkdir_p base_dir diff --git a/lib/autobuild/import/git.rb b/lib/autobuild/import/git.rb index f8d90bd3..023272e3 100644 --- a/lib/autobuild/import/git.rb +++ b/lib/autobuild/import/git.rb @@ -40,7 +40,8 @@ def default_alternates @default_alternates = cache_dirs.map do |path| File.join(File.expand_path(path), '%s') end - else Array.new + else + Array.new end end end @@ -764,7 +765,8 @@ def has_commit?(package, commit_id) rescue SubcommandFailed => e if e.status == 1 false - else raise + else + raise end end @@ -775,7 +777,8 @@ def has_branch?(package, branch_name) rescue SubcommandFailed => e if e.status == 1 false - else raise + else + raise end end @@ -1020,7 +1023,8 @@ def update_alternates(package) File.readlines(alternates_path) .map(&:strip) .find_all { |l| !l.empty? } - else Array.new + else + Array.new end alternates = each_alternate_path(package).map do |path| diff --git a/lib/autobuild/import/svn.rb b/lib/autobuild/import/svn.rb index c758c761..85941ff0 100644 --- a/lib/autobuild/import/svn.rb +++ b/lib/autobuild/import/svn.rb @@ -202,7 +202,8 @@ def svn_info(package) # Try svn upgrade and info again run_svn(package, 'upgrade', retry: false) svninfo = run_svn(package, 'info') - else raise + else + raise end end diff --git a/lib/autobuild/importer.rb b/lib/autobuild/importer.rb index afa2209c..b45fd390 100644 --- a/lib/autobuild/importer.rb +++ b/lib/autobuild/importer.rb @@ -357,7 +357,8 @@ def perform_update(package, only_local = false) message = Autobuild.color('interrupted', :red) if last_error raise last_error - else raise + else + raise end rescue ::Exception => e message = Autobuild.color('update failed', :red) @@ -410,7 +411,8 @@ def perform_checkout(package, **options) execute_post_hooks(package) rescue Interrupt if last_error then raise last_error - else raise + else + raise end rescue ::Exception => e last_error = e diff --git a/lib/autobuild/package.rb b/lib/autobuild/package.rb index 4af1188c..6fffe1fd 100644 --- a/lib/autobuild/package.rb +++ b/lib/autobuild/package.rb @@ -119,7 +119,8 @@ def installstamp def update? if @update.nil? Autobuild.do_update - else @update + else + @update end end @@ -492,7 +493,8 @@ def process_formatting_string(msg, *prefix_style) suffix << token.gsub(/%s/, name) elsif suffix.empty? prefix << token - else suffix << token + else + suffix << token end end if suffix.empty? diff --git a/lib/autobuild/packages/autotools.rb b/lib/autobuild/packages/autotools.rb index 0a9ff0f2..ab6baadd 100644 --- a/lib/autobuild/packages/autotools.rb +++ b/lib/autobuild/packages/autotools.rb @@ -206,7 +206,8 @@ def prepare varname, = o.split("=").first if (current_flag = testflags.find { |fl| fl =~ /^#{varname}=/ }) current_flag != o - else false + else + false end end end diff --git a/lib/autobuild/packages/cmake.rb b/lib/autobuild/packages/cmake.rb index d3fae24b..debc4f87 100644 --- a/lib/autobuild/packages/cmake.rb +++ b/lib/autobuild/packages/cmake.rb @@ -440,7 +440,8 @@ def configure def show_make_messages? if !@show_make_messages.nil? @show_make_messages - else CMake.show_make_messages? + else + CMake.show_make_messages? end end diff --git a/lib/autobuild/parallel.rb b/lib/autobuild/parallel.rb index f654c6fc..fde431ac 100644 --- a/lib/autobuild/parallel.rb +++ b/lib/autobuild/parallel.rb @@ -88,7 +88,8 @@ def push(task, base_priority = 1) if task.respond_to?(:package) started_packages[task.package] ||= -started_packages.size queue[task] = started_packages[task.package] - else queue[task] = base_priority + else + queue[task] = base_priority end end diff --git a/lib/autobuild/progress_display.rb b/lib/autobuild/progress_display.rb index 20fc634d..5365bdd7 100644 --- a/lib/autobuild/progress_display.rb +++ b/lib/autobuild/progress_display.rb @@ -246,7 +246,8 @@ def group_messages(messages) groups.last[1] = (current_group.first..group_end_index) groups << [prefix, [idx, other_idx]] grouping = true - else break + else + break end end end diff --git a/lib/autobuild/reporting.rb b/lib/autobuild/reporting.rb index 8f034116..bc95bfe0 100644 --- a/lib/autobuild/reporting.rb +++ b/lib/autobuild/reporting.rb @@ -130,7 +130,8 @@ def self.report(on_package_failures: default_report_on_package_failures) # :exit if debug is false, or :raise if it is true def self.default_report_on_package_failures if Autobuild.debug then :raise - else :exit + else + :exit end end @@ -163,7 +164,8 @@ def self.report_finish_on_error(errors, raise interrupted_by if interrupted_by e = if errors.size == 1 then errors.first - else CompositeException.new(errors) + else + CompositeException.new(errors) end raise e elsif %i[report_silent report].include?(on_package_failures) diff --git a/lib/autobuild/subcommand.rb b/lib/autobuild/subcommand.rb index b8da93e8..4739cee5 100644 --- a/lib/autobuild/subcommand.rb +++ b/lib/autobuild/subcommand.rb @@ -268,7 +268,8 @@ def self.run(target, phase, *command) end logdir = if target.respond_to?(:logdir) target.logdir - else Autobuild.logdir + else + Autobuild.logdir end if target.respond_to?(:working_directory) @@ -288,7 +289,8 @@ def self.run(target, phase, *command) open_flag = if Autobuild.keep_oldlogs then 'a' elsif Autobuild.registered_logfile?(logname) then 'a' - else 'w' + else + 'w' end open_flag << ":BINARY" @@ -318,9 +320,12 @@ def self.run(target, phase, *command) logfile.sync = true begin + # close_others: false is needed because some code (e.g. + # orogen/make) implicitly pass FDs to the child pid = spawn( env, *command, - chdir: chdir, in: inread, out: outwrite, err: outwrite + chdir: chdir, in: inread, out: outwrite, err: outwrite, + close_others: false ) rescue StandardError => e raise Failed.new(nil, false), e.message @@ -398,7 +403,8 @@ def self.run(target, phase, *command) logname, e.status, subcommand_output ) error.retry = if e.retry?.nil? then options[:retry] - else e.retry? + else + e.retry? end error.phase = phase raise error, e.message diff --git a/lib/autobuild/test_utility.rb b/lib/autobuild/test_utility.rb index f00a4ea4..c1ebaad6 100644 --- a/lib/autobuild/test_utility.rb +++ b/lib/autobuild/test_utility.rb @@ -24,7 +24,8 @@ def initialize(name, package, install_on_error: true) def coverage_enabled? if @coverage_enabled.nil? TestUtility.coverage_enabled? - else @coverage_enabled + else + @coverage_enabled end end @@ -53,7 +54,8 @@ def coverage_source_dir if @coverage_source_dir relative = if package.respond_to?(:builddir) package.builddir - else package.srcdir + else + package.srcdir end File.expand_path(@coverage_source_dir, relative) end From f1d916cc7cc0cf4f05a6eddd0dcff46bdfd1d17f Mon Sep 17 00:00:00 2001 From: Sylvain Date: Wed, 17 Nov 2021 17:41:12 -0300 Subject: [PATCH 3/3] fix: do not close stdin, older CMake versions fail when it is --- lib/autobuild/subcommand.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/autobuild/subcommand.rb b/lib/autobuild/subcommand.rb index 4739cee5..9e0842c3 100644 --- a/lib/autobuild/subcommand.rb +++ b/lib/autobuild/subcommand.rb @@ -306,7 +306,7 @@ def self.run(target, phase, *command) status = File.open(logname, open_flag) do |logfile| if input_streams.empty? - inread = :close + inread = :in else inread, inwrite = IO.pipe # to feed subprocess stdin end