Get suite running smoothly again (#824)

Am planning on proposing/implementing a new feature, but first I noticed
that there are a number of failing specs on main, so I figured I would
start with that.

As written, this includes four main adjustments, two of which might
actually be bugfixes:
- one spec was hanging indefinitely for me locally on a `Thread.join`
call - explicitly capturing the child threads and joining them directly
fixed it (might be machine dependent?)
- when an encoding is specified, the rails schema up to date hook was
passing arguments incorrectly (kwarg containing `encoding` was being
interpreted as the `length` positional arg) (possible bug)
- many specs were attempting to create file submodules without the
`protocol.file.allow=always` config option causing failure
- alias detection for amendment appeared to be broken, due to regexp
incompatibilities between ruby/git (git wasn't properly parsing `\s` in
the value matcher for `git config --regexp [key] [value]` command), so
moved that matching into ruby (possible bug, also maybe machine/git
build opt dependent)
- dropped ruby 2.6 from the test matrix - it appears that it's far
enough EOL that github has trouble even setting it up. I can attempt to
add it back/get it working, but I figured it might be time to ditch it

*I don't have a windows machine, so I'm not gonna be much help with
those CI builds (should note fails appear unrelated to anything I've
done here).

.github/workflows/tests.yml

@@ -14,7 +14,6 @@ jobs:
       fail-fast: false
       matrix:
         ruby-version:
-          - '2.6'
           - '2.7'
           - '3.0'
           - '3.1'

lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb

@@ -35,7 +35,7 @@ def run # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplex
     private
 
     def encoding
-      return unless @config.key?('encoding')
+      return {} unless @config.key?('encoding')
 
       { encoding: @config['encoding'] }.compact
     end
@@ -53,7 +53,7 @@ def schema_files
     end
 
     def schema
-      @schema ||= schema_files.map { |file| File.read(file, encoding) }.join
+      @schema ||= schema_files.map { |file| File.read(file, **encoding) }.join
       @schema.tr('_', '')
     end
 

lib/overcommit/hook_context/helpers/file_modifications.rb

@@ -12,7 +12,7 @@ def amendment?
         cmd = Overcommit::Utils.parent_command
         return unless cmd
 
-        amend_pattern = 'commit(\s.*)?\s--amend(\s|$)'
+        amend_pattern = /commit(\s.*)?\s--amend/
 
         # Since the ps command can return invalid byte sequences for commands
         # containing unicode characters, we replace the offending characters,
@@ -24,18 +24,11 @@ def amendment?
             encode('UTF-8')
         end
 
-        return @amendment if
-          # True if the command is a commit with the --amend flag
-          @amendment = !(/\s#{amend_pattern}/ =~ cmd).nil?
+        # True if the command is a commit with the --amend flag
+        return @amendment if @amendment = cmd.match?(amend_pattern)
 
         # Check for git aliases that call `commit --amend`
-        `git config --get-regexp "^alias\\." "#{amend_pattern}"`.
-          scan(/alias\.([-\w]+)/). # Extract the alias
-          each do |match|
-            return @amendment if
-              # True if the command uses a git alias for `commit --amend`
-              @amendment = !(/git(\.exe)?\s+#{match[0]}/ =~ cmd).nil?
-          end
+        return @amendment if @amendment = command_is_amend_alias?(cmd, amend_pattern)
 
         @amendment
       end
@@ -74,6 +67,22 @@ def modified_lines_in_file(file)
         end
         @modified_lines[file]
       end
+
+      private
+
+      def command_is_amend_alias?(cmd, amend_pattern)
+        `git config --get-regexp "^alias"`.split("\n").each do |alias_def|
+          alias_map = alias_def.match /alias\.(?<to>[-\w]+)\s+(?<from>.+)/
+          next unless alias_map
+
+          alias_from_match = alias_map[:from].match? amend_pattern
+          alias_to_match = cmd.match? /git(\.exe)?\s+#{alias_map[:to]}/
+
+          # True if the command uses a git alias for `commit --amend`
+          return true if @amendment = alias_from_match && alias_to_match
+        end
+        false
+      end
     end
   end
 end

spec/overcommit/git_repo_spec.rb

@@ -24,12 +24,13 @@
         end
 
         submodule = repo do
-          `git submodule add #{nested_submodule} nested-sub 2>&1 > #{File::NULL}`
+          git_config = '-c protocol.file.allow=always'
+          `git #{git_config} submodule add #{nested_submodule} nested-sub 2>&1 > #{File::NULL}`
           `git commit -m "Add nested submodule"`
         end
 
         repo do
-          `git submodule add #{submodule} sub 2>&1 > #{File::NULL}`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub 2>&1 > #{File::NULL}`
           example.run
         end
       end
@@ -150,7 +151,7 @@
         end
 
         before do
-          `git submodule add #{submodule} sub 2>&1 > #{File::NULL}`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub 2>&1 > #{File::NULL}`
         end
 
         it { should_not include File.expand_path('sub') }
@@ -178,7 +179,8 @@
           `git commit --allow-empty -m "Submodule commit"`
         end
 
-        `git submodule add #{submodule} #{submodule_dir} 2>&1 > #{File::NULL}`
+        git_config = '-c protocol.file.allow=always'
+        `git #{git_config} submodule add #{submodule} #{submodule_dir} 2>&1 > #{File::NULL}`
         `git commit -m "Add submodule"`
       end
 
@@ -282,7 +284,7 @@
         touch 'tracked'
         `git add tracked`
         `git commit -m "Initial commit"`
-        `git submodule add #{submodule} sub 2>&1 > #{File::NULL}`
+        `git -c protocol.file.allow=always submodule add #{submodule} sub 2>&1 > #{File::NULL}`
         touch 'staged'
         `git add staged`
         example.run
@@ -327,7 +329,7 @@
       end
 
       repo do
-        `git submodule add #{submodule} sub-repo 2>&1 > #{File::NULL}`
+        `git -c protocol.file.allow=always submodule add #{submodule} sub-repo 2>&1 > #{File::NULL}`
         `git commit -m "Initial commit"`
         example.run
       end
@@ -343,7 +345,8 @@
           `git commit --allow-empty -m "Another submodule"`
         end
 
-        `git submodule add #{another_submodule} another-sub-repo 2>&1 > #{File::NULL}`
+        git_config = '-c protocol.file.allow=always'
+        `git #{git_config} submodule add #{another_submodule} another-sub-repo 2>&1 > #{File::NULL}`
       end
 
       it { should be_empty }
@@ -365,11 +368,12 @@
 
     context 'when there are multiple submodule removals staged' do
       before do
-        another_submodule = repo do
+        another_submod = repo do
           `git commit --allow-empty -m "Another submodule"`
         end
 
-        `git submodule add #{another_submodule} yet-another-sub-repo 2>&1 > #{File::NULL}`
+        git_conf = '-c protocol.file.allow=always'
+        `git #{git_conf} submodule add #{another_submod} yet-another-sub-repo 2>&1 > #{File::NULL}`
         `git commit -m "Add yet another submodule"`
         `git rm sub-repo`
         `git rm yet-another-sub-repo`

spec/overcommit/hook/prepare_commit_msg/base_spec.rb

@@ -38,9 +38,7 @@
           contents + "bravo\n"
         end
       end
-      Thread.new { hook_1.run }
-      Thread.new { hook_2.run }
-      Thread.list.each { |t| t.join unless t == Thread.current }
+      [Thread.new { hook_1.run }, Thread.new { hook_2.run }].each(&:join)
       expect(File.read(tempfile)).to match(/alpha\n#{initial_content}bravo\n/m)
     end
   end

spec/overcommit/hook_context/commit_msg_spec.rb

@@ -298,7 +298,7 @@
         end
 
         repo do
-          `git submodule add #{submodule} sub > #{File::NULL} 2>&1`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1`
           `git commit -m "Add submodule"`
           echo('Hello World', 'sub/submodule-file')
           `git submodule foreach "git add submodule-file" < #{File::NULL}`
@@ -474,7 +474,7 @@
         end
 
         repo do
-          `git submodule add #{submodule} sub > #{File::NULL} 2>&1`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1`
           `git commit -m "Add submodule"`
           echo('Hello World', 'sub/submodule-file')
           `git submodule foreach "git add submodule-file" < #{File::NULL}`
@@ -500,7 +500,7 @@
         end
 
         repo do
-          `git submodule add #{submodule} sub > #{File::NULL} 2>&1`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1`
           `git commit -m "Add submodule"`
           echo('Hello World', 'sub/submodule-file')
           `git submodule foreach "git add submodule-file" < #{File::NULL}`
@@ -532,7 +532,7 @@
         end
 
         repo do
-          `git submodule add #{submodule} sub > #{File::NULL} 2>&1`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1`
           `git commit -m "Add submodule"`
           `git rm sub`
           example.run
@@ -561,7 +561,7 @@
       end
 
       repo do
-        `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+        `git -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
         expect(subject).to_not include File.expand_path('test-sub')
       end
     end

spec/overcommit/hook_context/post_checkout_spec.rb

@@ -67,7 +67,7 @@
 
       repo do
         `git commit --allow-empty -m "Initial commit"`
-        `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+        `git -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
         `git commit -m "Add submodule"`
         expect(subject).to_not include File.expand_path('test-sub')
       end

spec/overcommit/hook_context/post_commit_spec.rb

@@ -20,7 +20,7 @@
       end
 
       repo do
-        `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+        `git -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
         `git commit -m "Initial commit"`
         expect(subject).to_not include File.expand_path('test-sub')
       end

spec/overcommit/hook_context/post_merge_spec.rb

@@ -94,7 +94,7 @@
       repo do
         `git commit --allow-empty -m "Initial commit"`
         `git checkout -b child > #{File::NULL} 2>&1`
-        `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+        `git -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
         `git commit -m "Add submodule"`
         `git checkout master > #{File::NULL} 2>&1`
         `git merge --no-ff --no-edit child`

spec/overcommit/hook_context/post_rewrite_spec.rb

@@ -101,8 +101,9 @@
         end
 
         repo do
+          git_config = '-c protocol.file.allow=always'
           `git commit --allow-empty -m "Initial commit"`
-          `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+          `git #{git_config} submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
           `git commit --amend -m "Add submodule"`
           expect(subject).to_not include File.expand_path('test-sub')
         end

spec/overcommit/hook_context/pre_commit_spec.rb

@@ -207,7 +207,7 @@
         end
 
         repo do
-          `git submodule add #{submodule} sub > #{File::NULL} 2>&1`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1`
           `git commit -m "Add submodule"`
           echo('Hello World', 'sub/submodule-file')
           `git submodule foreach "git add submodule-file" < #{File::NULL}`
@@ -383,7 +383,7 @@
         end
 
         repo do
-          `git submodule add #{submodule} sub > #{File::NULL} 2>&1`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1`
           `git commit -m "Add submodule"`
           echo('Hello World', 'sub/submodule-file')
           `git submodule foreach "git add submodule-file" < #{File::NULL}`
@@ -409,7 +409,7 @@
         end
 
         repo do
-          `git submodule add #{submodule} sub > #{File::NULL} 2>&1`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1`
           `git commit -m "Add submodule"`
           echo('Hello World', 'sub/submodule-file')
           `git submodule foreach "git add submodule-file" < #{File::NULL}`
@@ -441,7 +441,7 @@
         end
 
         repo do
-          `git submodule add #{submodule} sub > #{File::NULL} 2>&1`
+          `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1`
           `git commit -m "Add submodule"`
           `git rm sub`
           example.run
@@ -470,7 +470,7 @@
       end
 
       repo do
-        `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+        `git -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
         expect(subject).to_not include File.expand_path('test-sub')
       end
     end

spec/overcommit/hook_context/run_all_spec.rb

@@ -46,7 +46,8 @@
         end
 
         repo do
-          `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
+          git_config = '-c protocol.file.allow=always'
+          `git #{git_config} submodule add #{submodule} test-sub 2>&1 > #{File::NULL}`
           example.run
         end
       end

spec/overcommit/installer_spec.rb

@@ -231,7 +231,7 @@ def hook_files_installed?(hooks_dir)
       context 'which has an external git dir' do
         let(:submodule) { File.join(target, 'submodule') }
         before do
-          system 'git', 'submodule', 'add', target, 'submodule',
+          system 'git', '-c', 'protocol.file.allow=always', 'submodule', 'add', target, 'submodule',
                  chdir: target, out: :close, err: :close
         end
         let(:submodule_git_file) { File.join(submodule, '.git') }