From 34150d64cb11bed73ef9a4f87d341e5d5b176734 Mon Sep 17 00:00:00 2001 From: Sami Samhuri Date: Mon, 17 Jan 2022 18:54:52 -0800 Subject: [PATCH] Extract job control to a separate class --- ruby/.rubocop.yml | 2 +- ruby/Gemfile.lock | 2 +- ruby/builtins.rb | 6 +-- ruby/colours.rb | 25 ++++++--- ruby/job_control.rb | 82 +++++++++++++++++++++++++++++ ruby/logger.rb | 2 + ruby/main.rb | 123 ++++++++++---------------------------------- 7 files changed, 135 insertions(+), 107 deletions(-) create mode 100644 ruby/job_control.rb diff --git a/ruby/.rubocop.yml b/ruby/.rubocop.yml index cb4c688..c58f192 100644 --- a/ruby/.rubocop.yml +++ b/ruby/.rubocop.yml @@ -11,7 +11,7 @@ Metrics/AbcSize: Max: 25 Metrics/MethodLength: - Max: 20 + Max: 30 Style/Documentation: Enabled: false diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock index e5224fe..a3e7c5f 100644 --- a/ruby/Gemfile.lock +++ b/ruby/Gemfile.lock @@ -27,7 +27,7 @@ PLATFORMS arm64-darwin-21 DEPENDENCIES - rubocop + rubocop (= 1.24.1) wordexp (~> 0.1) RUBY VERSION diff --git a/ruby/builtins.rb b/ruby/builtins.rb index 4df5efc..61b4b60 100644 --- a/ruby/builtins.rb +++ b/ruby/builtins.rb @@ -2,8 +2,8 @@ class Shell class Builtins attr_reader :logger - def initialize(shell, logger = Logger.instance) - @shell = shell + def initialize(job_control, logger = Logger.instance) + @job_control = job_control @logger = logger end @@ -21,7 +21,7 @@ class Shell def builtin_bg(args) cmd = args.shift - @shell.exec_command(cmd, args, background: true) + @job_control.exec_command(cmd, args, background: true) end def builtin_cd(args) diff --git a/ruby/colours.rb b/ruby/colours.rb index a48cbbe..118e7b4 100644 --- a/ruby/colours.rb +++ b/ruby/colours.rb @@ -1,9 +1,20 @@ class Shell - # These colours should be safe on dark and light backgrounds. - BLUE = "\033[1;34m".freeze - GREEN = "\033[1;32m".freeze - YELLOW = "\033[1;33m".freeze - RED = "\033[1;31m".freeze - WHITE = "\033[1;37m".freeze - CLEAR = "\033[0;m".freeze + module Colours + # These colours should be safe on dark and light backgrounds. + BLUE = "\033[1;34m".freeze + GREEN = "\033[1;32m".freeze + YELLOW = "\033[1;33m".freeze + RED = "\033[1;31m".freeze + WHITE = "\033[1;37m".freeze + CLEAR = "\033[0;m".freeze + end + + def self.included(other) + Colours.constants.each do |sym| + next if sym == :CLEAR + other.define_method(sym.to_s.downcase) do |text| + "#{Colours.const_get(sym)}#{text}#{CLEAR}" + end + end + end end diff --git a/ruby/job_control.rb b/ruby/job_control.rb new file mode 100644 index 0000000..975730a --- /dev/null +++ b/ruby/job_control.rb @@ -0,0 +1,82 @@ +require './colours' +require './job' + +class Shell + class JobControl + include Colours + + attr_reader :logger + + def initialize(logger = Logger.instance) + @jobs_by_pid = {} + @logger = logger + end + + def trap_sigchld + # handler for SIGCHLD when a child's state changes + Signal.trap('CHLD') do |_signo| + pid = Process.waitpid(-1, Process::WNOHANG) + if pid.nil? + # no-op + elsif (job = @jobs_by_pid[pid]) + puts "\n#{YELLOW}#{job.id}#{CLEAR}: " \ + "#{WHITE}(pid #{pid})#{CLEAR} " \ + "#{GREEN}#{job.cmd}#{CLEAR} " \ + "#{job.args.inspect}" + else + warn "\n#{YELLOW}[WARN]#{CLEAR} No job found for child with PID #{pid}" + end + end + end + + def exec_command(cmd, args, background: false) + unless (path = resolve_executable(cmd)) + warn "#{RED}[ERROR]#{CLEAR} command not found: #{cmd}" + return -2 + end + + pid = fork { exec([path, cmd], *args) } + if background + job = Job.new(next_job_id, pid, cmd, args) + @jobs_by_pid[pid] = job + puts "Background job #{job.id} (pid #{pid})" + Process.waitpid(pid, Process::WNOHANG) + 0 + else + begin + Process.waitpid(pid) + $CHILD_STATUS.exitstatus + rescue Errno::ECHILD => e + # FIXME: why does this happen? doesn't seem to be a real problem + logger.verbose "#{YELLOW}#{e.message}#{CLEAR} but child was just forked 🧐" + 0 + end + end + rescue StandardError => e + warn "#{RED}[ERROR]#{CLEAR} #{e.message} #{e.inspect}" + -5 + end + + # Return absolute and relative paths directly, or searches PATH for a + # matching executable with the given filename and returns its path. + # Returns nil when no such command was found. + def resolve_executable(path_or_filename) + # process absolute and relative paths directly + return path_or_filename if path_or_filename['/'] && \ + File.executable?(path_or_filename) + + filename = path_or_filename + ENV['PATH'].split(':').each do |dir| + path = File.join(dir, filename) + next unless File.exist?(path) + return path if File.executable?(path) + logger.warn "Found #{path} but it's not executable" + end + nil + end + + def next_job_id + (@jobs_by_pid.values.map(&:id).max || 0) + 1 + end + end +end diff --git a/ruby/logger.rb b/ruby/logger.rb index fab0a21..07c8ab7 100644 --- a/ruby/logger.rb +++ b/ruby/logger.rb @@ -4,6 +4,8 @@ class Shell # Queues up messages to be printed out when readline is waiting for input, to prevent # mixing shell output with command output. class Logger + include Colours + Log = Struct.new(:level, :message) attr_reader :logs diff --git a/ruby/main.rb b/ruby/main.rb index 0f048f4..7aeca42 100755 --- a/ruby/main.rb +++ b/ruby/main.rb @@ -1,42 +1,50 @@ #!/usr/bin/env ruby -w require 'English' -require 'open3' require 'readline' require 'wordexp' require './builtins' require './colours' +require './job_control' require './logger' -require './job' +# TODO: change to module after extracting all or most of the code class Shell - attr_reader :logger, :options + include Colours - def initialize(args = ARGV) - @builtins = Builtins.new(self) - @jobs_by_pid = {} - @logger = Logger.instance + attr_reader :builtins, :job_control, :logger, :options + + def initialize(args: ARGV, builtins: nil, job_control: nil, logger: nil) + logger ||= Logger.instance + job_control ||= JobControl.new + builtins ||= Builtins.new(job_control) + @builtins = builtins + @job_control = job_control + @logger = logger @options = parse_options(args) logger.verbose "Options: #{options.inspect}" end def main - trap_sigchld if options[:command] logger.verbose "Executing command: #{options[:command]}" print_logs exit process_command(options[:command]) - elsif $stdin.isatty - add_to_history = true - status = 0 - loop do - print_logs - print "#{RED}#{status}#{CLEAR} " unless status.zero? - line = Readline.readline(prompt(Dir.pwd), add_to_history) - Readline::HISTORY.pop if line.nil? || line.strip.empty? - status = process_command(line) - end + end + repl if $stdin.isatty + end + + def repl + @job_control.trap_sigchld + add_to_history = true + status = 0 + loop do + print_logs + print "#{RED}#{status}#{CLEAR} " unless status.zero? + line = Readline.readline(prompt(Dir.pwd), add_to_history) + Readline::HISTORY.pop if line.nil? || line.strip.empty? + status = process_command(line) end end @@ -63,23 +71,6 @@ class Shell options end - def trap_sigchld - # handler for SIGCHLD when a child's state changes - Signal.trap('CHLD') do |_signo| - pid = Process.waitpid(-1, Process::WNOHANG) - if pid.nil? - # no-op - elsif (job = @jobs_by_pid[pid]) - puts "\n#{YELLOW}#{job.id}#{CLEAR}: " \ - "#{WHITE}(pid #{pid})#{CLEAR} " \ - "#{GREEN}#{job.cmd}#{CLEAR} " \ - "#{job.args.inspect}" - else - warn "\n#{YELLOW}[WARN]#{CLEAR} No job found for child with PID #{pid}" - end - end - end - def print_logs logger.logs.each do |log| message = "#{log.message}#{CLEAR}" @@ -106,70 +97,12 @@ class Shell @builtins.exec(cmd, args) else logger.verbose "Shelling out for #{cmd}" - exec_command(cmd, args) + @job_control.exec_command(cmd, args) end rescue Errno => e warn "#{RED}[ERROR]#{CLEAR} #{e.message}" -1 end - - def exec_command(cmd, args, background: false) - unless (path = resolve_executable(cmd)) - warn "#{RED}[ERROR]#{CLEAR} command not found: #{cmd}" - return -2 - end - - pid = fork - if pid - # parent - if background - job = Job.new(next_job_id, pid, cmd, args) - @jobs_by_pid[pid] = job - puts "Background job #{job.id} (pid #{pid})" - Process.waitpid(pid, Process::WNOHANG) - 0 - else - begin - Process.waitpid(pid) - $CHILD_STATUS.exitstatus - rescue Errno::ECHILD => e - # FIXME: why does this happen? doesn't seem to be a real problem - logger.verbose "#{YELLOW}#{e.message}#{CLEAR} but child was just forked 🧐" - 0 - end - end - else - # child - exec([path, cmd], *args) - # if we make it here then exec failed - -4 - end - rescue StandardError => e - warn "#{RED}[ERROR]#{CLEAR} #{e.message} #{e.inspect}" - -5 - end - - # Return absolute and relative paths directly, or searches PATH for a - # matching executable with the given filename and returns its path. - # Returns nil when no such command was found. - def resolve_executable(path_or_filename) - # process absolute and relative paths directly - return path_or_filename if path_or_filename['/'] && \ - File.executable?(path_or_filename) - - filename = path_or_filename - ENV['PATH'].split(':').each do |dir| - path = File.join(dir, filename) - next unless File.exist?(path) - return path if File.executable?(path) - logger.warn "Found #{path} but it's not executable" - end - nil - end - - def next_job_id - (@jobs_by_pid.values.map(&:id).max || 0) + 1 - end end -Shell.new(ARGV).main if $PROGRAM_NAME == __FILE__ +Shell.new(args: ARGV).main if $PROGRAM_NAME == __FILE__