You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by bu...@apache.org on 2022/01/15 20:42:16 UTC

[hbase] branch branch-2.4 updated: HBASE-26469 correct HBase shell exit behavior to match code passed to exit (#4018)

This is an automated email from the ASF dual-hosted git repository.

busbey pushed a commit to branch branch-2.4
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2.4 by this push:
     new 8b370f7  HBASE-26469 correct HBase shell exit behavior to match code passed to exit (#4018)
8b370f7 is described below

commit 8b370f7f89935cb91a07a1c0210de587cea2ab9c
Author: Sean Busbey <bu...@apache.org>
AuthorDate: Tue Sep 28 23:00:11 2021 -0500

    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. Note that the below can be summarized as lining up with
    behavior from earlier HBase releases. This behavior changes in future 2.5+ releases of HBase and
    you should see the jira for more details.
    * exit called in an initialization script will result in a non-zero exit code iff a non-zero code
      is passed to the exit call.
    * when reading user input from stdin, a call to exit will result in a 0 exit code if the shell
      is interactive, and a non-zero exit code if the shell is non-interactive. any optional code
      passed to the call to exit won't change wether the exit code is zero or non-zero.
    
    Signed-off-by: Peter Somogyi <ps...@apache.org>
    (cherry picked from commit 821e6a36cc88c2348cda6af8d9eb8a526054dffc)
---
 hbase-shell/src/main/ruby/irb/hirb.rb           | 40 +++++++++++-
 hbase-shell/src/main/ruby/jar-bootstrap.rb      | 83 ++++++++++---------------
 hbase-shell/src/main/ruby/shell.rb              | 40 +++++-------
 hbase-shell/src/main/ruby/shell/hbase_loader.rb | 56 -----------------
 hbase-shell/src/test/ruby/shell/shell_test.rb   | 19 ------
 5 files changed, 87 insertions(+), 151 deletions(-)

diff --git a/hbase-shell/src/main/ruby/irb/hirb.rb b/hbase-shell/src/main/ruby/irb/hirb.rb
index f8eed2a..33419b9 100644
--- a/hbase-shell/src/main/ruby/irb/hirb.rb
+++ b/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
diff --git a/hbase-shell/src/main/ruby/jar-bootstrap.rb b/hbase-shell/src/main/ruby/jar-bootstrap.rb
index ef3727d..6ccded5 100644
--- a/hbase-shell/src/main/ruby/jar-bootstrap.rb
+++ b/hbase-shell/src/main/ruby/jar-bootstrap.rb
@@ -87,6 +87,7 @@ script2run = nil
 log_level = org.apache.log4j.Level::ERROR
 @shell_debug = false
 interactive = true
+full_backtrace = false
 top_level_definitions = false
 _configuration = nil
 D_ARG = '-D'.freeze
@@ -104,7 +105,7 @@ while (arg = ARGV.shift)
     found.push(arg)
   elsif arg == '-d' || arg == '--debug'
     log_level = org.apache.log4j.Level::DEBUG
-    $fullBackTrace = true
+    full_backtrace = true
     @shell_debug = true
     found.push(arg)
     puts 'Setting DEBUG log level...'
@@ -183,61 +184,43 @@ 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
+# Wrapping this check in the exception handler is only present in the 2.4.x line so that we can
+# maintain behavior that matches the earlier 2.x releases
+::Shell::Shell.exception_handler(!full_backtrace) do
+  exit @shell.exit_code unless interactive || @shell.exit_code.nil?
 end
diff --git a/hbase-shell/src/main/ruby/shell.rb b/hbase-shell/src/main/ruby/shell.rb
index 6daf0de..f7efa34 100644
--- a/hbase-shell/src/main/ruby/shell.rb
+++ b/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,34 +314,14 @@ 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
     #
     # @param [Boolean] hide_traceback if true, Exceptions will be converted to
diff --git a/hbase-shell/src/main/ruby/shell/hbase_loader.rb b/hbase-shell/src/main/ruby/shell/hbase_loader.rb
deleted file mode 100644
index 2ad2ea9..0000000
--- a/hbase-shell/src/main/ruby/shell/hbase_loader.rb
+++ /dev/null
@@ -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
diff --git a/hbase-shell/src/test/ruby/shell/shell_test.rb b/hbase-shell/src/test/ruby/shell/shell_test.rb
index fcc9c61..d6c60fd 100644
--- a/hbase-shell/src/test/ruby/shell/shell_test.rb
+++ b/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