Redesign RubyGit::FileHelpers.which

lib/ruby_git.rb

@@ -76,15 +76,15 @@ def self.open(worktree_path)
   #    => "/Users/jsmith"
   #   worktree = Worktree.clone('https://github.com/main-branch/ruby_git.git')
   #   worktree.path
-  #     => "/Users/jsmith/ruby_git"
+  #    => "/Users/jsmith/ruby_git"
   #
   # @example Using a specified worktree_path
   #   FileUtils.pwd
   #    => "/Users/jsmith"
   #   worktree_path = '/tmp/project'
   #   worktree = Worktree.clone('https://github.com/main-branch/ruby_git.git', to_path: worktree_path)
   #   worktree.path
-  #     => "/tmp/project"
+  #    => "/tmp/project"
   #
   # @param [String] repository_url a reference to a Git repository
   #

lib/ruby_git/file_helpers.rb

@@ -10,33 +10,61 @@ module FileHelpers
     #
     # Works for both Linux/Unix and Windows.
     #
-    # @example Searching over the PATH for a command
-    #   path = FileUtils.which('git')
+    # @example Searching over the ENV['PATH'] for a command
+    #   RubyGit::FileHelpers.which('git')
+    #    => #<Pathname:/usr/local/bin/git>
     #
-    # @example Overriding the default PATH
-    #   path = FileUtils.which('git', ['/usr/bin', '/usr/local/bin'])
+    # @example Overriding the default path (which is ENV['PATH'])
+    #   RubyGit::FileHelpers.which('git', path: '/usr/bin:/usr/local/bin')
+    #    => #<Pathname:/usr/bin/git>
     #
-    # @param [String] cmd The basename of the executable file to search for
-    # @param [Array<String>] paths The list of directories to search for basename in
-    # @param [Array<String>] exts The list of extensions that indicate that a file is executable
+    # @example On Windows
+    #   RubyGit::FileHelpers.which('git', path: 'C:\Windows\System32;C:\Program Files\Git\bin')
+    #    => #<Pathname:C:/Program Files/Git/bin/git.exe>
     #
-    # `exts` is for Windows. Other platforms should accept the default.
+    # @param [String] cmd_basename The basename of the executable file to search for
+    #
+    # @param [String] path The list of directories to search for basename in given as a String
+    #
+    # The default value is ENV['PATH']. The string is split on `File::PATH_SEPARATOR`. If the `path` is an
+    # empty string, RuntimeError is raised.
+    #
+    # @param [String] path_ext The list of extensions to check for
+    #
+    # The default value is ENV['PATHEXT'].  The string is split on `File::PATH_SEPARATOR`. `path_ext` may
+    # be an empty string to indicate no extensions should be added to `cmd` when searching the `path`.
+    #
+    # Typically this is only used for Windows to specify binary file extensions such as `.EXE;.BAT;.CMD`
     #
     # @return [Pathname,nil] The path to the first executable file found on the path or
     #   nil an executable file was not found.
     #
-    def self.which(
-      cmd,
-      paths: ENV['PATH'].split(File::PATH_SEPARATOR),
-      exts: (ENV['PATHEXT']&.split(';') || [''])
-    )
-      raise 'PATH is not set' unless ENV.keys.include?('PATH')
+    def self.which(cmd_basename, path: ENV['PATH'], path_ext: ENV['PATHEXT'])
+      raise 'path can not be nil or empty' if path.nil? || path.empty?
 
-      paths
-        .product(exts)
-        .map { |path, ext| Pathname.new(File.join(path, "#{cmd}#{ext}")) }
-        .reject { |path| path.directory? || !path.executable? }
-        .find { |exe_path| !exe_path.directory? && exe_path.executable? }
+      split_path(path)
+        .product(split_path(path_ext))
+        .each do |path_dir, ext|
+          cmd_pathname = File.join(path_dir, "#{cmd_basename}#{ext}")
+          return Pathname.new(cmd_pathname) if !File.directory?(cmd_pathname) && File.executable?(cmd_pathname)
+        end
+      nil
+    end
+
+    # Split the path string on the File::PATH_SEPARATOR
+    #
+    # @example
+    #   File::PATH_SEPARATOR
+    #    => ":"
+    #   FileHelpers.split_path('/bin:/usr/local/bin')
+    #    => ["/bin", "/usr/local/bin"]
+    #
+    # @param [String] path The path string to split
+    #
+    # @return [Array<String>] the split path or [''] if the path was nil
+    #
+    def self.split_path(path)
+      path&.split(File::PATH_SEPARATOR) || ['']
     end
   end
 end

lib/ruby_git/git_binary.rb

@@ -42,7 +42,8 @@ def path=(path)
     #
     # @example Get the git found on the PATH
     #   git = RubyGit::GitBinary.new
-    #   path = git.path
+    #   git.path
+    #    => #<Pathname:/usr/bin/git>
     #
     # @return [Pathname] the path to the git binary
     #
@@ -57,7 +58,8 @@ def path
     #
     # @example Find the pathname to `super_git`
     #   git = RubyGit::GitBinary.new
-    #   git.path = git.default_path(basename: 'super_git')
+    #   git.default_path(basename: 'super_git')
+    #    => #<Pathname:/usr/bin/super_git>
     #
     # @param [String] basename The basename of the git command
     #
@@ -66,15 +68,17 @@ def path
     # @raise [RuntimeError] if either PATH is not set or an executable file
     #   `basename` was not found on the path.
     #
-    def self.default_path(basename: 'git')
-      RubyGit::FileHelpers.which(basename) || raise("Could not find '#{basename}' in the PATH.")
+    def self.default_path(basename: 'git', path: ENV['PATH'], path_ext: ENV['PATHEXT'])
+      RubyGit::FileHelpers.which(basename, path: path, path_ext: path_ext) ||
+        raise("Could not find '#{basename}' in the PATH.")
     end
 
     # The version of git referred to by the path
     #
     # @example for version 2.28.0
     #   git = RubyGit::GitBinary.new
-    #   puts git.version #=> [2,28,0]
+    #   git.version
+    #    => [2, 28, 0]
     #
     # @return [Array<Integer>] an array of integers representing the version.
     #

lib/ruby_git/worktree.rb

@@ -76,15 +76,15 @@ def self.open(worktree_path)
     #    => "/Users/jsmith"
     #   worktree = Worktree.clone('https://github.com/main-branch/ruby_git.git')
     #   worktree.path
-    #     => "/Users/jsmith/ruby_git"
+    #    => "/Users/jsmith/ruby_git"
     #
     # @example Using a specified worktree_path
     #   FileUtils.pwd
     #    => "/Users/jsmith"
     #   worktree_path = '/tmp/project'
     #   worktree = Worktree.clone('https://github.com/main-branch/ruby_git.git', to_path: worktree_path)
     #   worktree.path
-    #     => "/tmp/project"
+    #    => "/tmp/project"
     #
     # @param [String] repository_url a reference to a Git repository
     #
@@ -110,6 +110,7 @@ def self.clone(repository_url, to_path: '')
 
     # Create a Worktree object
     # @api private
+    #
     def initialize(worktree_path)
       raise RubyGit::Error, "Path '#{worktree_path}' not valid." unless File.directory?(worktree_path)
 

spec/lib/ruby_git/file_helpers_spec.rb

@@ -0,0 +1,130 @@
+# frozen_string_literal: true
+
+require 'tmpdir'
+
+RSpec.describe RubyGit::FileHelpers do
+  describe '.which' do
+    subject { described_class.which(command, path: path, path_ext: path_ext) }
+    let(:command) { 'command' }
+    let(:path) { nil } # Equivalent to PATH not set in ENV
+    let(:path_ext) { nil } # Equivalent to PATHEXT not set in ENV
+
+    let(:root_dir) { Dir.mktmpdir }
+    after { FileUtils.rm_rf root_dir }
+
+    context 'when PATH is not set in ENV' do
+      it 'should raise RuntimeError' do
+        expect { subject }.to raise_error(RuntimeError)
+      end
+    end
+
+    context "when path is '/usr/bin:/usr/local/bin' and path_ext is nil" do
+      let(:path_dir_1) { File.join(root_dir, 'usr', 'bin') }
+      let(:path_dir_2) { File.join(root_dir, 'usr', 'local', 'bin') }
+      let(:path) { [path_dir_1, path_dir_2].join(File::PATH_SEPARATOR) }
+
+      context 'and command is not found in the path' do
+        it { is_expected.to be_nil }
+      end
+
+      context 'and /usr/local/bin/command is NOT an executable file' do
+        let(:command_dir) { path_dir_1 }
+        let(:command_path) { File.join(command_dir, command) }
+        before do
+          FileUtils.mkdir_p(command_dir)
+          FileUtils.touch(command_path)
+        end
+
+        it { is_expected.to be_nil }
+      end
+
+      context 'and /usr/local/bin/command is a directory' do
+        let(:command_dir) { path_dir_1 }
+        let(:command_path) { File.join(command_dir, command) }
+        before do
+          FileUtils.mkdir_p(command_dir)
+          FileUtils.mkdir(command_path)
+        end
+
+        it { is_expected.to be_nil }
+      end
+
+      context 'and /usr/bin/command is an executable file' do
+        let(:command_dir) { path_dir_1 }
+        let(:command_path) { File.join(command_dir, command) }
+        before do
+          FileUtils.mkdir_p(command_dir)
+          FileUtils.touch(command_path)
+          FileUtils.chmod(0o755, command_path)
+        end
+
+        it { is_expected.to eq(Pathname.new(command_path)) }
+      end
+
+      context 'and /usr/local/bin/command is an executable file' do
+        let(:command_dir) { path_dir_2 }
+        let(:command_path) { File.join(command_dir, command) }
+        before do
+          FileUtils.mkdir_p(command_dir)
+          FileUtils.touch(command_path)
+          FileUtils.chmod(0o755, command_path)
+        end
+
+        it { is_expected.to eq(Pathname.new(command_path)) }
+      end
+
+      context 'and /usr/local/bin/command is a symlink to an executable file' do
+        let(:command_dir) { path_dir_2 }
+        let(:command_path) { File.join(command_dir, command) }
+        let(:actual_command_path) { File.join(command_dir, "actual_#{command}") }
+        before do
+          FileUtils.mkdir_p(command_dir)
+          FileUtils.touch(actual_command_path)
+          FileUtils.chmod(0o755, actual_command_path)
+          FileUtils.ln_s(actual_command_path, command_path)
+        end
+
+        it { is_expected.to eq(Pathname.new(command_path)) }
+      end
+
+      context 'and both /usr/bin/command and /usr/local/bin/command are executable files' do
+        let(:command_dir_1) { path_dir_1 }
+        let(:command_path_1) { File.join(command_dir_1, command) }
+        before do
+          FileUtils.mkdir_p(command_dir_1)
+          FileUtils.touch(command_path_1)
+          FileUtils.chmod(0o755, command_path_1)
+        end
+
+        let(:command_dir_2) { path_dir_2 }
+        let(:command_path_2) { File.join(command_dir_2, command) }
+        before do
+          FileUtils.mkdir_p(command_dir_2)
+          FileUtils.touch(command_path_2)
+          FileUtils.chmod(0o755, command_path_2)
+        end
+
+        it { is_expected.to eq(Pathname.new(command_path_1)) }
+      end
+
+      context "and path_ext is '.EXE:.BAT:.CMD'" do
+        let(:path_dir_1) { File.join(root_dir, 'usr', 'bin') }
+        let(:path_dir_2) { File.join(root_dir, 'usr', 'local', 'bin') }
+        let(:path) { [path_dir_1, path_dir_2].join(File::PATH_SEPARATOR) }
+        let(:path_ext) { %w[.EXE .BAT .CMD].join(File::PATH_SEPARATOR) }
+
+        context 'and /usr/local/bin/command.BAT is an executable file' do
+          let(:command_dir) { path_dir_1 }
+          let(:command_path) { File.join(command_dir, "#{command}.BAT") }
+          before do
+            FileUtils.mkdir_p(command_dir)
+            FileUtils.touch(command_path)
+            FileUtils.chmod(0o755, command_path)
+          end
+
+          it { is_expected.to eq(Pathname.new(File.join(root_dir, '/usr/bin/command.BAT'))) }
+        end
+      end
+    end
+  end
+end