HBASE-26469 correct HBase shell exit behavior to match code passed to exit (#4018)

* refactors how we handle running the passed in initialization script to make use of IRB sessions
  directly instead of reimplementing things ourselves
* simplify how we initialize our IRB config
* insert a shim for capturing exit codes passed via user calls to exit
* make use of user provided exit code unless we're reading stdin in interactive mode

This changes the exit code of the shell
* a 0 return code, or no return code, passed to a call to exit from stdin in non-interactive mode
  will now exit cleanly. in prior versions this would have exitted with an error and non-zero exit
  code.
* for other combinations of passing in an initilization script or reading from stdin with using the
  non-interactive flag, the exit code being 0 or non-0 should now line up with releases prior to
  2.4.z, which is a change in behavior compared to 2.4.z.

Signed-off-by: Peter Somogyi <psomogyi@apache.org>
(cherry picked from commit 821e6a36cc)

hbase-shell/src/main/ruby/irb/hirb.rb

@@ -23,7 +23,7 @@ module IRB
 
   # Subclass of IRB so can intercept methods
   class HIRB < Irb
-    def initialize
+    def initialize(workspace = nil, input_method = nil)
       # This is ugly.  Our 'help' method above provokes the following message
       # on irb construction: 'irb: warn: can't alias help from irb_help.'
       # Below, we reset the output so its pointed at /dev/null during irb
@@ -44,7 +44,7 @@ module IRB
       # The stderr is an input to stty to re-adjust the terminal for the error('stdin isnt a terminal')
       # incase the command is piped with hbase shell(eg - >echo 'list' | bin/hbase shell)
       `stty icrnl <&2`
-      super
+      super(workspace, input_method)
     ensure
       f.close
       $stdout = STDOUT
@@ -57,4 +57,40 @@ module IRB
       super unless @context.last_value.nil?
     end
   end
+
+  ##
+  # HBaseLoader serves a similar purpose to IRB::IrbLoader, but with a different separation of
+  # concerns. This loader allows us to directly get the path for a filename in ruby's load path,
+  # and then use that in IRB::Irb
+  module HBaseLoader
+    ##
+    # Determine the loadable path for a given filename by searching through $LOAD_PATH
+    #
+    # This serves a similar purpose to IRB::IrbLoader#search_file_from_ruby_path, but uses JRuby's
+    # loader, which allows us to find special paths like "uri:classloader" inside of a Jar.
+    #
+    # @param [String] filename
+    # @return [String] path
+    def self.path_for_load(filename)
+      return File.absolute_path(filename) if File.exist? filename
+
+      # Get JRuby's LoadService from the global (singleton) instance of the runtime
+      # (org.jruby.Ruby), which allows us to use JRuby's tools for searching the load path.
+      runtime = org.jruby.Ruby.getGlobalRuntime
+      loader = runtime.getLoadService
+      search_state = loader.findFileForLoad filename
+      raise LoadError, "no such file to load -- #{filename}" if search_state.library.nil?
+
+      search_state.loadName
+    end
+
+    ##
+    # Return a file handle for the given file found in the load path
+    #
+    # @param [String] filename
+    # @return [FileInputMethod] InputMethod for passing to IRB session
+    def self.file_for_load(filename)
+      FileInputMethod.new(File.new(path_for_load(filename)))
+    end
+  end
 end

hbase-shell/src/main/ruby/jar-bootstrap.rb

@@ -105,6 +105,7 @@ script2run = nil
 log_level = org.apache.log4j.Level::ERROR
 @shell_debug = false
 interactive = true
+full_backtrace = false
 top_level_definitions = false
 
 opts.each do |opt, arg|
@@ -116,7 +117,7 @@ opts.each do |opt, arg|
     conf_from_cli = add_to_configuration(conf_from_cli, arg)
   when '--debug'
     log_level = org.apache.log4j.Level::DEBUG
-    $fullBackTrace = true
+    full_backtrace = true
     @shell_debug = true
     puts 'Setting DEBUG log level...'
   when '--noninteractive'
@@ -185,61 +186,39 @@ end
 # instance variables (@hbase and @shell) onto Ruby's top-level receiver object known as "main".
 @shell.export_all(self) if top_level_definitions
 
+require 'irb'
+require 'irb/ext/change-ws'
+require 'irb/hirb'
+
+# Configure IRB
+IRB.setup(nil)
+IRB.conf[:PROMPT][:CUSTOM] = {
+  PROMPT_I: '%N:%03n:%i> ',
+  PROMPT_S: '%N:%03n:%i%l ',
+  PROMPT_C: '%N:%03n:%i* ',
+  RETURN: "=> %s\n"
+}
+
+IRB.conf[:IRB_NAME] = 'hbase'
+IRB.conf[:AP_NAME] = 'hbase'
+IRB.conf[:PROMPT_MODE] = :CUSTOM
+IRB.conf[:BACK_TRACE_LIMIT] = 0 unless full_backtrace
+
+# Create a workspace we'll use across sessions.
+workspace = @shell.get_workspace
+
 # If script2run, try running it.  If we're in interactive mode, will go on to run the shell unless
 # script calls 'exit' or 'exit 0' or 'exit errcode'.
-require 'shell/hbase_loader'
 if script2run
-  ::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(Hbase::Loader.file_for_load(script2run), filename = script2run) }
+  ::Shell::Shell.exception_handler(!full_backtrace) do
+    IRB::HIRB.new(workspace, IRB::HBaseLoader.file_for_load(script2run)).run
+  end
+  exit @shell.exit_code unless @shell.exit_code.nil?
 end
 
-# If we are not running interactively, evaluate standard input
-::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(STDIN) } unless interactive
-
 if interactive
   # Output a banner message that tells users where to go for help
   @shell.print_banner
-
-  require 'irb'
-  require 'irb/ext/change-ws'
-  require 'irb/hirb'
-
-  module IRB
-    # Override of the default IRB.start
-    def self.start(ap_path = nil)
-      $0 = File.basename(ap_path, '.rb') if ap_path
-
-      IRB.setup(ap_path)
-      IRB.conf[:PROMPT][:CUSTOM] = {
-        :PROMPT_I => "%N:%03n:%i> ",
-        :PROMPT_S => "%N:%03n:%i%l ",
-        :PROMPT_C => "%N:%03n:%i* ",
-        :RETURN => "=> %s\n"
-      }
-
-      @CONF[:IRB_NAME] = 'hbase'
-      @CONF[:AP_NAME] = 'hbase'
-      @CONF[:PROMPT_MODE] = :CUSTOM
-      @CONF[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
-
-      hirb = if @CONF[:SCRIPT]
-               HIRB.new(nil, @CONF[:SCRIPT])
-             else
-               HIRB.new
-             end
-
-      shl = TOPLEVEL_BINDING.receiver.instance_variable_get :'@shell'
-      hirb.context.change_workspace shl.get_workspace
-
-      @CONF[:IRB_RC].call(hirb.context) if @CONF[:IRB_RC]
-      # Storing our current HBase IRB Context as the main context is imperative for several reasons,
-      # including auto-completion.
-      @CONF[:MAIN_CONTEXT] = hirb.context
-
-      catch(:IRB_EXIT) do
-        hirb.eval_input
-      end
-    end
-  end
-
-  IRB.start
 end
+IRB::HIRB.new(workspace).run
+exit @shell.exit_code unless interactive || @shell.exit_code.nil?

hbase-shell/src/main/ruby/shell.rb

@@ -100,6 +100,18 @@ module Shell
     @debug = false
     attr_accessor :debug
 
+    # keep track of the passed exit code. nil means never called.
+    @exit_code = nil
+    attr_accessor :exit_code
+
+    alias __exit__ exit
+    # exit the interactive shell and save that this
+    # happend via a call to exit
+    def exit(ret = 0)
+      @exit_code = ret
+      IRB.irb_exit(IRB.CurrentContext.irb, ret)
+    end
+
     def initialize(hbase, interactive = true)
       self.hbase = hbase
       self.interactive = interactive
@@ -302,33 +314,13 @@ For more on the HBase Shell, see http://hbase.apache.org/book.html
       # Install all the hbase commands, constants, and instance variables @shell and @hbase. This
       # will override names that conflict with IRB methods like "help".
       export_all(hbase_receiver)
+      # make it so calling exit will hit our pass-through rather than going directly to IRB
+      hbase_receiver.send :define_singleton_method, :exit, lambda { |rc = 0|
+        @shell.exit(rc)
+      }
       ::IRB::WorkSpace.new(hbase_receiver.get_binding)
     end
 
-    ##
-    # Read from an instance of Ruby's IO class and evaluate each line within the shell's workspace
-    #
-    # Unlike Ruby's require or load, this method allows us to execute code with a custom binding. In
-    # this case, we are using the binding constructed with all the HBase shell constants and
-    # methods.
-    #
-    # @param [IO] io instance of Ruby's IO (or subclass like File) to read script from
-    # @param [String] filename to print in tracebacks
-    def eval_io(io, filename = 'stdin')
-      require 'irb/ruby-lex'
-      # Mixing HBaseIOExtensions into IO allows us to pass IO objects to RubyLex.
-      IO.include HBaseIOExtensions
-
-      workspace = get_workspace
-      scanner = RubyLex.new
-      scanner.set_input(io)
-
-      scanner.each_top_level_statement do |statement, linenum|
-        puts(workspace.evaluate(nil, statement, filename, linenum))
-      end
-      nil
-    end
-
     ##
     # Runs a block and logs exception from both Ruby and Java, optionally discarding the traceback
     #

hbase-shell/src/main/ruby/shell/hbase_loader.rb

@@ -1,56 +0,0 @@
-#
-#
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements.  See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership.  The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License.  You may obtain a copy of the License at
-#
-#     http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-
-module Hbase
-  ##
-  # HBase::Loader serves a similar purpose to IRB::IrbLoader, but with a different separation of
-  # concerns. This loader allows us to directly get the path for a filename in ruby's load path,
-  # and then use that in combination with something like HBase::Shell#eval_io.
-  module Loader
-    ##
-    # Determine the loadable path for a given filename by searching through $LOAD_PATH
-    #
-    # This serves a similar purpose to IRB::IrbLoader#search_file_from_ruby_path, but uses JRuby's
-    # loader, which allows us to find special paths like "uri:classloader" inside of a Jar.
-    #
-    # @param [String] filename
-    # @return [String] path
-    def self.path_for_load(filename)
-      return File.absolute_path(filename) if File.exist? filename
-
-      # Get JRuby's LoadService from the global (singleton) instance of the runtime
-      # (org.jruby.Ruby), which allows us to use JRuby's tools for searching the load path.
-      runtime = org.jruby.Ruby.getGlobalRuntime
-      loader = runtime.getLoadService
-      search_state = loader.findFileForLoad filename
-      raise LoadError, "no such file to load -- #{filename}" if search_state.library.nil?
-
-      search_state.loadName
-    end
-
-    ##
-    # Return a file handle for the given file found in the load path
-    #
-    # @param [String] filename
-    # @return [File] file handle
-    def self.file_for_load(filename)
-      File.new(path_for_load(filename))
-    end
-  end
-end

hbase-shell/src/test/ruby/shell/shell_test.rb

@@ -113,25 +113,6 @@ class ShellTest < Test::Unit::TestCase
 
   #-----------------------------------------------------------------------------
 
-  define_test 'Shell::Shell#eval_io should evaluate IO' do
-    StringIO.include HBaseIOExtensions
-    # check that at least one of the commands is present while evaluating
-    io = StringIO.new <<~EOF
-      puts (respond_to? :list)
-    EOF
-    output = capture_stdout { @shell.eval_io(io) }
-    assert_match(/true/, output)
-
-    # check that at least one of the HBase constants is present while evaluating
-    io = StringIO.new <<~EOF
-      ROWPREFIXFILTER.dump
-    EOF
-    output = capture_stdout { @shell.eval_io(io) }
-    assert_match(/"ROWPREFIXFILTER"/, output)
-  end
-
-  #-----------------------------------------------------------------------------
-
   define_test 'Shell::Shell#exception_handler should hide traceback' do
     class TestException < RuntimeError; end
     # When hide_traceback is true, exception_handler should replace exceptions