Merge pull request #16 from BaxterStockman/safe-ps4

PS4 with subshells causes erroneous extra hits to be registered

Author: infertux

.gitignore

@@ -13,6 +13,7 @@ doc/
 lib/bundler/man
 pkg
 rdoc
+.rubocop-*
 spec/reports
 test/tmp
 test/version_tmp

.rubocop.yml

@@ -1,27 +1,6 @@
-Metrics/AbcSize:
-  Enabled: false
-
-Metrics/MethodLength:
-  Enabled: false
-
-Metrics/LineLength:
-  Max: 120
-  Exclude: [spec/**/*]
-
-Style/AccessModifierIndentation:
-  EnforcedStyle: outdent
-
-Style/AndOr:
-  EnforcedStyle: conditionals
-
-Style/SignalException:
-  EnforcedStyle: only_raise
+inherit_from:
+  - https://gist.githubusercontent.com/infertux/cdd2ccc6e0a0cd94f458/raw
 
 Style/SpecialGlobalVars:
   Enabled: false
 
-Style/StringLiterals:
-  EnforcedStyle: double_quotes
-
-Style/TrailingComma:
-  Enabled: false

README.md

@@ -20,10 +20,11 @@
 [Dependencies]: https://gemnasium.com/infertux/bashcov "Bashcov dependencies on Gemnasium"
 [Bashcov]: https://github.com/infertux/bashcov
 [SimpleCov]: https://github.com/colszowka/simplecov "Bashcov is backed by SimpleCov to generate awesome coverage report"
+[Test app demo]: http://infertux.github.com/bashcov/test_app/ "Coverage for the bundled test application"
 
 You should check out these coverage examples - it's worth a thousand words:
 
-  - [Test app demo](http://infertux.github.com/bashcov/test_app/ "Coverage for the bundled test application")
+  - [Test app demo]
   - [RVM demo](http://infertux.github.com/bashcov/rvm/ "Coverage for RVM")
 
 ## Installation
@@ -51,6 +52,52 @@ Open `./coverage/index.html` to browse the coverage report.
 You can take great advantage of [SimpleCov] by adding a `.simplecov` file in your project's root (like [this](https://github.com/infertux/bashcov/blob/master/spec/test_app/.simplecov)).
 See [SimpleCov README](https://github.com/colszowka/simplecov#readme) for more information.
 
+### Some gory details
+
+Figuring out where an executing Bash script lives in the file system can be
+surprisingly difficult.  Bash offers a fair amount of [introspection into its
+internals](https://www.gnu.org/software/bash/manual/html_node/Bash-Variables.html),
+but the location of the current script has to be inferred from the limited
+information available through `BASH_SOURCE`, `PWD`, and `OLDPWD` (and
+potentially `DIRSTACK` if you are using `pushd`/`popd`).  For this purpose,
+Bashcov puts Bash in debug mode and sets up a `PS4` that expands the values of
+these variables, reading them on each command that Bash executes.  But, given
+that:
+
+  * `BASH_SOURCE` is only an absolute path if the script was invoked using an
+    absolute path,
+  * The builtins `cd`, `pushd`, and `popd` alter `PWD` and `OLDPWD`, and
+  * None of these variables are read-only and can therefore be `unset` or
+    otherwise altered,
+
+it can be easy to lose track of where we are.
+
+"Wait a minute, what about `pwd`, `readlink`, and so on?"  That would be great,
+except that subshells executed as part of expanding the `PS4` can cause Bash to
+report [extra executions](https://github.com/infertux/bashcov/commit/4130874e30a05b7ab6ea66fb96a19acaa973c178)
+for [certain lines](https://github.com/infertux/bashcov/pull/16).  Also,
+subshells are slow, and the `PS4` is expanded on each and every command when
+Bash is in debug mode.
+
+To deal with these limitations, Bashcov uses the expedient of maintaining two
+stacks that track changes to `PWD` and `OLDPWD`.  To determine the full path to
+the executing script, Bashcov iterates in reverse over the `PWD` stack, testing
+for the first `$PWD/$BASH_SOURCE` combination that refers to an existing file.
+This heuristic is susceptible to false positives -- under certain combinations
+of directory stucture, script invocation paths, and working directory changes,
+it may yield a path that doesn't refer to the currently-running script.
+However, it performs well under the various working directory changes performed
+in the [test app demo] and avoids the spurious extra hits caused by using
+subshells in the `PS4`.
+
+One final note on innards: Bash 4.3 fixed a bug in which `PS4` expansion is
+truncated to a maximum of 128 characters.  On platforms whose Bash version
+suffers from this bug, Bashcov uses the ASCII record separator character to
+delimit the `PS4` fields, whereas it uses a long random string on Bash 4.3 and
+above.  When the field delimiter appears in the path of a script under test or
+in a command the script executes, Bashcov won't correctly parse the `PS4` and
+will abort early with incomplete coverage results.
+
 ## Contributing
 
 Bug reports and patches are most welcome.

lib/bashcov.rb

@@ -1,48 +1,71 @@
 require "optparse"
 require "ostruct"
-require "bashcov/version"
-require "bashcov/lexer"
-require "bashcov/line"
+require "pathname"
+
+require "bashcov/bash_info"
 require "bashcov/runner"
-require "bashcov/xtrace"
+require "bashcov/version"
 
 # Bashcov default module
 # @note Keep it short!
 module Bashcov
-  class << self
-    # @return [OpenStruct] Bashcov settings
-    attr_reader :options
-
-    # @return [String] The project's root directory
-    def root_directory
-      @root_directory ||= Dir.pwd
-    end
+  extend Bashcov::BashInfo
 
-    # Sets default options overriding any existing ones.
-    # @return [void]
-    def set_default_options!
-      @options ||= OpenStruct.new
-      @options.skip_uncovered = false
-      @options.mute = false
+  class << self
+    # @return [OpenStruct] The +OpenStruct+ object representing Bashcov's
+    # execution environment
+    def options
+      set_default_options! unless defined?(@options)
+      @options
     end
 
     # Parses the given CLI arguments and sets +options+.
     # @param [Array] args list of arguments
     # @raise [SystemExit] if invalid arguments are given
     # @return [void]
     def parse_options!(args)
-      option_parser.parse!(args)
+      begin
+        option_parser.parse!(args)
+      rescue OptionParser::ParseError, Errno::ENOENT => e
+        abort "#{option_parser.program_name}: #{e.message}"
+      end
 
       if args.empty?
         abort("You must give exactly one command to execute.")
       else
-        @options.command = args.join(" ")
+        options.command = args.unshift(bash_path)
       end
     end
 
+    # @return [String] Program name
+    def program_name
+      "bashcov"
+    end
+
     # @return [String] Program name including version for easy consistent output
-    def name
-      "bashcov v#{VERSION}"
+    # @note +fullname+ instead of name to avoid clashing with +Module.name+
+    def fullname
+      "#{program_name} v#{VERSION}"
+    end
+
+    # Wipe the current options and reset default values
+    def set_default_options!
+      @options = OpenStruct.new
+
+      @options.root_directory   = Dir.getwd
+      @options.skip_uncovered   = false
+      @options.bash_path        = "/bin/bash"
+      @options.mute             = false
+    end
+
+    # Passes off +respond_to?+ to {options} for missing methods
+    def respond_to_missing?(*args)
+      options.respond_to?(*args)
+    end
+
+    # Dispatches missing methods to {options}
+    def method_missing(method_name, *args, &block)
+      options.send(method_name, *args, &block)
     end
 
   private
@@ -60,17 +83,25 @@ def help(program_name)
 
     def option_parser
       OptionParser.new do |opts|
-        opts.program_name = "bashcov"
+        opts.program_name = program_name
         opts.version = Bashcov::VERSION
         opts.banner = help opts.program_name
 
         opts.separator "\nSpecific options:"
 
         opts.on("-s", "--skip-uncovered", "Do not report uncovered files") do |s|
-          @options.skip_uncovered = s
+          options.skip_uncovered = s
         end
         opts.on("-m", "--mute", "Do not print script output") do |m|
-          @options.mute = m
+          options.mute = m
+        end
+        opts.on("--bash-path PATH", "Path to Bash executable") do |p|
+          raise Errno::ENOENT, p unless File.file? p
+          options.bash_path = p
+        end
+        opts.on("--root PATH", "Project root directory") do |d|
+          raise Errno::ENOENT, d unless File.directory? d
+          options.root_directory = d
         end
 
         opts.separator "\nCommon options:"
@@ -86,6 +117,3 @@ def option_parser
     end
   end
 end
-
-# Make sure default options are set
-Bashcov.set_default_options!

lib/bashcov/bash_info.rb

@@ -0,0 +1,30 @@
+module Bashcov
+  # Module exposing information concerning the installed Bash version
+  # @note methods do not cache results because +bash_path+ can change at
+  #   runtime
+  # @note receiver is expected to implement +bash_path+
+  module BashInfo
+    # @return [Array<String>] An array representing the components of
+    #   +BASH_VERSINFO+
+    def bash_versinfo
+      `#{bash_path} -c 'echo "${BASH_VERSINFO[@]}"'`.chomp.split
+    end
+
+    # @return [Boolean] Whether Bash supports +BASH_XTRACEFD+
+    def bash_xtracefd?
+      bash_versinfo[0..1].join.to_i >= 41
+    end
+
+    # @param [Integer] length  the number of bytes to test; default 128
+    # @return [Boolean] whether Bash supports a +PS4+ of at least a given
+    #   number of bytes
+    # @see https://tiswww.case.edu/php/chet/bash/CHANGES
+    # @note Item +i.+ under the +bash-4.2-release+ to +bash-4.3-alpha+ change
+    #   list notes that version 4.2 truncates +PS4+ if it is greater than 128
+    #   bytes.
+    def truncated_ps4?(length = 128)
+      ps4 = SecureRandom.base64(length)
+      !`PS4=#{ps4} #{bash_path} 2>&1 1>&- -xc 'echo hello'`.start_with?(ps4)
+    end
+  end
+end

lib/bashcov/errors.rb

@@ -0,0 +1,14 @@
+module Bashcov
+  # Signals an error parsing Bash's debugging output.
+  class XtraceError < ::RuntimeError
+    # Will contain the coverages parsed prior to the error
+    attr_reader :files
+
+    # @param [#to_s]  message An error message
+    # @param [Hash]   files   A hash containing coverage information
+    def initialize(message, files)
+      @files = files
+      super(message)
+    end
+  end
+end

lib/bashcov/field_stream.rb

@@ -0,0 +1,73 @@
+module Bashcov
+  # Classes for streaming token-delimited fields
+  class FieldStream
+    attr_accessor :read
+
+    # @param [IO] read an IO object opened for reading
+    def initialize(read = nil)
+      @read = read
+    end
+
+    # A convenience wrapper around +each_line(sep)+ that also does
+    # +chomp(sep)+ on the yielded line.
+    # @param [String, nil] delim the field separator for the stream
+    # @return [void]
+    # @yieldparam [String] field each +chomp+ed line
+    def each_field(delim)
+      return enum_for(__method__, delim) unless block_given?
+
+      read.each_line(delim) do |line|
+        yield line.chomp(delim)
+      end
+    end
+
+    # Yields fields extracted from a input stream
+    # @param [String, nil] delim   the field separator
+    # @param [Integer] field_count the number of fields to extract
+    # @param [Regexp] start_match  a +Regexp+ that, when matched against the
+    #   input stream, signifies the beginning of the next series of fields to
+    #   yield
+    # @yieldparam [String] field each field extracted from the stream.  If
+    #   +start_match+ is matched with fewer than +field_count+ fields yielded
+    #   since the last match, yields empty strings until +field_count+ is
+    #   reached.
+    def each(delim, field_count, start_match)
+      return enum_for(__method__, delim, field_count, start_match) unless block_given?
+
+      # Whether the current field is the start-of-fields match
+      matched_start = nil
+
+      # The number of fields processed since passing the last start-of-fields
+      # match
+      seen_fields = 0
+
+      fields = each_field(delim)
+
+      # Close over +field_count+ and +seen_fields+ to yield empty strings to
+      # the caller when we've already hit the next start-of-fields match
+      yield_remaining = -> { (field_count - seen_fields).times { yield "" } }
+
+      # Advance until the first start-of-fields match
+      loop { break if fields.next =~ start_match }
+
+      fields.each do |field|
+        # If the current field is the start-of-fields match...
+        if field =~ start_match
+          # Fill out any remaining (unparseable) fields with empty strings
+          yield_remaining.call
+
+          matched_start = nil
+          seen_fields = 0
+        elsif seen_fields < field_count
+            yield field
+            seen_fields += 1
+        end
+      end
+
+      # One last filling-out of empty fields if we're at the end of the stream
+      yield_remaining.call
+
+      read.close unless read.closed?
+    end
+  end
+end

lib/bashcov/lexer.rb

@@ -1,21 +1,23 @@
+require "bashcov/line"
+
 module Bashcov
   # Simple lexer which analyzes Bash files in order to get information for
   # coverage
   class Lexer
     # Lines starting with one of these tokens are irrelevant for coverage
-    IGNORE_START_WITH = %w(# function)
+    IGNORE_START_WITH = %w(# function).freeze
 
     # Lines ending with one of these tokens are irrelevant for coverage
-    IGNORE_END_WITH = %w|(|
+    IGNORE_END_WITH = %w|(|.freeze
 
     # Lines containing only one of these keywords are irrelevant for coverage
-    IGNORE_IS = %w(esac if then else elif fi while do done { } ;;)
+    IGNORE_IS = %w(esac if then else elif fi while do done { } ;;).freeze
 
     # @param [String] filename File to analyze
     # @param [Hash] coverage Coverage with executed lines marked
     # @raise [ArgumentError] if the given +filename+ is invalid.
     def initialize(filename, coverage)
-      @filename = File.expand_path(filename)
+      @filename = filename
       @coverage = coverage
 
       raise ArgumentError, "#{@filename} is not a file" unless File.file?(@filename)