From 607ce057b760b697dc63623e7b7dc06a79316eab Mon Sep 17 00:00:00 2001 From: benmelz Date: Sat, 24 Feb 2024 18:50:30 -0500 Subject: [PATCH 1/3] 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 | 1 - .../pre_commit/rails_schema_up_to_date.rb | 4 +-- .../helpers/file_modifications.rb | 31 ++++++++++++------- spec/overcommit/git_repo_spec.rb | 22 +++++++------ .../hook/prepare_commit_msg/base_spec.rb | 4 +-- .../hook_context/commit_msg_spec.rb | 10 +++--- .../hook_context/post_checkout_spec.rb | 2 +- .../hook_context/post_commit_spec.rb | 2 +- .../hook_context/post_merge_spec.rb | 2 +- .../hook_context/post_rewrite_spec.rb | 3 +- .../hook_context/pre_commit_spec.rb | 10 +++--- spec/overcommit/hook_context/run_all_spec.rb | 3 +- spec/overcommit/installer_spec.rb | 2 +- 13 files changed, 54 insertions(+), 42 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 26c20dd1..71a83ba6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,7 +14,6 @@ jobs: fail-fast: false matrix: ruby-version: - - '2.6' - '2.7' - '3.0' - '3.1' diff --git a/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb b/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb index dafd31a1..1f2de970 100644 --- a/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb +++ b/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 diff --git a/lib/overcommit/hook_context/helpers/file_modifications.rb b/lib/overcommit/hook_context/helpers/file_modifications.rb index 00aaccc8..8d221ddc 100644 --- a/lib/overcommit/hook_context/helpers/file_modifications.rb +++ b/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\.(?[-\w]+)\s+(?.+)/ + 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 diff --git a/spec/overcommit/git_repo_spec.rb b/spec/overcommit/git_repo_spec.rb index 128737c3..08f3b585 100644 --- a/spec/overcommit/git_repo_spec.rb +++ b/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` diff --git a/spec/overcommit/hook/prepare_commit_msg/base_spec.rb b/spec/overcommit/hook/prepare_commit_msg/base_spec.rb index 616d6b8e..ef635a48 100644 --- a/spec/overcommit/hook/prepare_commit_msg/base_spec.rb +++ b/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 diff --git a/spec/overcommit/hook_context/commit_msg_spec.rb b/spec/overcommit/hook_context/commit_msg_spec.rb index 2aa68516..058f1508 100644 --- a/spec/overcommit/hook_context/commit_msg_spec.rb +++ b/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 diff --git a/spec/overcommit/hook_context/post_checkout_spec.rb b/spec/overcommit/hook_context/post_checkout_spec.rb index 50657c37..df1dccd2 100644 --- a/spec/overcommit/hook_context/post_checkout_spec.rb +++ b/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 diff --git a/spec/overcommit/hook_context/post_commit_spec.rb b/spec/overcommit/hook_context/post_commit_spec.rb index 81758256..5322a1f2 100644 --- a/spec/overcommit/hook_context/post_commit_spec.rb +++ b/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 diff --git a/spec/overcommit/hook_context/post_merge_spec.rb b/spec/overcommit/hook_context/post_merge_spec.rb index aa89fa82..0621b695 100644 --- a/spec/overcommit/hook_context/post_merge_spec.rb +++ b/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` diff --git a/spec/overcommit/hook_context/post_rewrite_spec.rb b/spec/overcommit/hook_context/post_rewrite_spec.rb index d2f18f6e..0d510c4a 100644 --- a/spec/overcommit/hook_context/post_rewrite_spec.rb +++ b/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 diff --git a/spec/overcommit/hook_context/pre_commit_spec.rb b/spec/overcommit/hook_context/pre_commit_spec.rb index dae42e66..719982a1 100644 --- a/spec/overcommit/hook_context/pre_commit_spec.rb +++ b/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 diff --git a/spec/overcommit/hook_context/run_all_spec.rb b/spec/overcommit/hook_context/run_all_spec.rb index 516fb70b..c6696edc 100644 --- a/spec/overcommit/hook_context/run_all_spec.rb +++ b/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 diff --git a/spec/overcommit/installer_spec.rb b/spec/overcommit/installer_spec.rb index c7cfb05e..8739a70e 100644 --- a/spec/overcommit/installer_spec.rb +++ b/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') } From 65e211451816710afe3b530d2e319db08f2c2f1b Mon Sep 17 00:00:00 2001 From: Shane da Silva Date: Sat, 24 Feb 2024 15:57:56 -0800 Subject: [PATCH 2/3] Revert "Get suite running smoothly again (#824)" (#840) This reverts commit 607ce057b760b697dc63623e7b7dc06a79316eab. Going to try to merge a different branch. --- .github/workflows/tests.yml | 1 + .../pre_commit/rails_schema_up_to_date.rb | 4 +-- .../helpers/file_modifications.rb | 31 +++++++------------ spec/overcommit/git_repo_spec.rb | 22 ++++++------- .../hook/prepare_commit_msg/base_spec.rb | 4 ++- .../hook_context/commit_msg_spec.rb | 10 +++--- .../hook_context/post_checkout_spec.rb | 2 +- .../hook_context/post_commit_spec.rb | 2 +- .../hook_context/post_merge_spec.rb | 2 +- .../hook_context/post_rewrite_spec.rb | 3 +- .../hook_context/pre_commit_spec.rb | 10 +++--- spec/overcommit/hook_context/run_all_spec.rb | 3 +- spec/overcommit/installer_spec.rb | 2 +- 13 files changed, 42 insertions(+), 54 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 71a83ba6..26c20dd1 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,6 +14,7 @@ jobs: fail-fast: false matrix: ruby-version: + - '2.6' - '2.7' - '3.0' - '3.1' diff --git a/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb b/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb index 1f2de970..dafd31a1 100644 --- a/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb +++ b/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 diff --git a/lib/overcommit/hook_context/helpers/file_modifications.rb b/lib/overcommit/hook_context/helpers/file_modifications.rb index 8d221ddc..00aaccc8 100644 --- a/lib/overcommit/hook_context/helpers/file_modifications.rb +++ b/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/ + amend_pattern = 'commit(\s.*)?\s--amend(\s|$)' # Since the ps command can return invalid byte sequences for commands # containing unicode characters, we replace the offending characters, @@ -24,11 +24,18 @@ def amendment? encode('UTF-8') end - # True if the command is a commit with the --amend flag - return @amendment if @amendment = cmd.match?(amend_pattern) + return @amendment if + # True if the command is a commit with the --amend flag + @amendment = !(/\s#{amend_pattern}/ =~ cmd).nil? # Check for git aliases that call `commit --amend` - return @amendment if @amendment = command_is_amend_alias?(cmd, amend_pattern) + `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 @amendment end @@ -67,22 +74,6 @@ 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\.(?[-\w]+)\s+(?.+)/ - 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 diff --git a/spec/overcommit/git_repo_spec.rb b/spec/overcommit/git_repo_spec.rb index 08f3b585..128737c3 100644 --- a/spec/overcommit/git_repo_spec.rb +++ b/spec/overcommit/git_repo_spec.rb @@ -24,13 +24,12 @@ end submodule = repo do - git_config = '-c protocol.file.allow=always' - `git #{git_config} submodule add #{nested_submodule} nested-sub 2>&1 > #{File::NULL}` + `git submodule add #{nested_submodule} nested-sub 2>&1 > #{File::NULL}` `git commit -m "Add nested submodule"` end repo do - `git -c protocol.file.allow=always submodule add #{submodule} sub 2>&1 > #{File::NULL}` + `git submodule add #{submodule} sub 2>&1 > #{File::NULL}` example.run end end @@ -151,7 +150,7 @@ end before do - `git -c protocol.file.allow=always submodule add #{submodule} sub 2>&1 > #{File::NULL}` + `git submodule add #{submodule} sub 2>&1 > #{File::NULL}` end it { should_not include File.expand_path('sub') } @@ -179,8 +178,7 @@ `git commit --allow-empty -m "Submodule commit"` end - git_config = '-c protocol.file.allow=always' - `git #{git_config} submodule add #{submodule} #{submodule_dir} 2>&1 > #{File::NULL}` + `git submodule add #{submodule} #{submodule_dir} 2>&1 > #{File::NULL}` `git commit -m "Add submodule"` end @@ -284,7 +282,7 @@ touch 'tracked' `git add tracked` `git commit -m "Initial commit"` - `git -c protocol.file.allow=always submodule add #{submodule} sub 2>&1 > #{File::NULL}` + `git submodule add #{submodule} sub 2>&1 > #{File::NULL}` touch 'staged' `git add staged` example.run @@ -329,7 +327,7 @@ end repo do - `git -c protocol.file.allow=always submodule add #{submodule} sub-repo 2>&1 > #{File::NULL}` + `git submodule add #{submodule} sub-repo 2>&1 > #{File::NULL}` `git commit -m "Initial commit"` example.run end @@ -345,8 +343,7 @@ `git commit --allow-empty -m "Another submodule"` end - git_config = '-c protocol.file.allow=always' - `git #{git_config} submodule add #{another_submodule} another-sub-repo 2>&1 > #{File::NULL}` + `git submodule add #{another_submodule} another-sub-repo 2>&1 > #{File::NULL}` end it { should be_empty } @@ -368,12 +365,11 @@ context 'when there are multiple submodule removals staged' do before do - another_submod = repo do + another_submodule = repo do `git commit --allow-empty -m "Another submodule"` end - git_conf = '-c protocol.file.allow=always' - `git #{git_conf} submodule add #{another_submod} yet-another-sub-repo 2>&1 > #{File::NULL}` + `git submodule add #{another_submodule} 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` diff --git a/spec/overcommit/hook/prepare_commit_msg/base_spec.rb b/spec/overcommit/hook/prepare_commit_msg/base_spec.rb index ef635a48..616d6b8e 100644 --- a/spec/overcommit/hook/prepare_commit_msg/base_spec.rb +++ b/spec/overcommit/hook/prepare_commit_msg/base_spec.rb @@ -38,7 +38,9 @@ contents + "bravo\n" end end - [Thread.new { hook_1.run }, Thread.new { hook_2.run }].each(&:join) + Thread.new { hook_1.run } + Thread.new { hook_2.run } + Thread.list.each { |t| t.join unless t == Thread.current } expect(File.read(tempfile)).to match(/alpha\n#{initial_content}bravo\n/m) end end diff --git a/spec/overcommit/hook_context/commit_msg_spec.rb b/spec/overcommit/hook_context/commit_msg_spec.rb index 058f1508..2aa68516 100644 --- a/spec/overcommit/hook_context/commit_msg_spec.rb +++ b/spec/overcommit/hook_context/commit_msg_spec.rb @@ -298,7 +298,7 @@ end repo do - `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1` + `git 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 -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1` + `git 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 -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1` + `git 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 -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1` + `git 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 -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` + `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` expect(subject).to_not include File.expand_path('test-sub') end end diff --git a/spec/overcommit/hook_context/post_checkout_spec.rb b/spec/overcommit/hook_context/post_checkout_spec.rb index df1dccd2..50657c37 100644 --- a/spec/overcommit/hook_context/post_checkout_spec.rb +++ b/spec/overcommit/hook_context/post_checkout_spec.rb @@ -67,7 +67,7 @@ repo do `git commit --allow-empty -m "Initial commit"` - `git -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` + `git 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 diff --git a/spec/overcommit/hook_context/post_commit_spec.rb b/spec/overcommit/hook_context/post_commit_spec.rb index 5322a1f2..81758256 100644 --- a/spec/overcommit/hook_context/post_commit_spec.rb +++ b/spec/overcommit/hook_context/post_commit_spec.rb @@ -20,7 +20,7 @@ end repo do - `git -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` + `git 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 diff --git a/spec/overcommit/hook_context/post_merge_spec.rb b/spec/overcommit/hook_context/post_merge_spec.rb index 0621b695..aa89fa82 100644 --- a/spec/overcommit/hook_context/post_merge_spec.rb +++ b/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 -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` + `git 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` diff --git a/spec/overcommit/hook_context/post_rewrite_spec.rb b/spec/overcommit/hook_context/post_rewrite_spec.rb index 0d510c4a..d2f18f6e 100644 --- a/spec/overcommit/hook_context/post_rewrite_spec.rb +++ b/spec/overcommit/hook_context/post_rewrite_spec.rb @@ -101,9 +101,8 @@ end repo do - git_config = '-c protocol.file.allow=always' `git commit --allow-empty -m "Initial commit"` - `git #{git_config} submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` + `git 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 diff --git a/spec/overcommit/hook_context/pre_commit_spec.rb b/spec/overcommit/hook_context/pre_commit_spec.rb index 719982a1..dae42e66 100644 --- a/spec/overcommit/hook_context/pre_commit_spec.rb +++ b/spec/overcommit/hook_context/pre_commit_spec.rb @@ -207,7 +207,7 @@ end repo do - `git -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1` + `git 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 -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1` + `git 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 -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1` + `git 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 -c protocol.file.allow=always submodule add #{submodule} sub > #{File::NULL} 2>&1` + `git 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 -c protocol.file.allow=always submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` + `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` expect(subject).to_not include File.expand_path('test-sub') end end diff --git a/spec/overcommit/hook_context/run_all_spec.rb b/spec/overcommit/hook_context/run_all_spec.rb index c6696edc..516fb70b 100644 --- a/spec/overcommit/hook_context/run_all_spec.rb +++ b/spec/overcommit/hook_context/run_all_spec.rb @@ -46,8 +46,7 @@ end repo do - git_config = '-c protocol.file.allow=always' - `git #{git_config} submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` + `git submodule add #{submodule} test-sub 2>&1 > #{File::NULL}` example.run end end diff --git a/spec/overcommit/installer_spec.rb b/spec/overcommit/installer_spec.rb index 8739a70e..c7cfb05e 100644 --- a/spec/overcommit/installer_spec.rb +++ b/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', '-c', 'protocol.file.allow=always', 'submodule', 'add', target, 'submodule', + system 'git', 'submodule', 'add', target, 'submodule', chdir: target, out: :close, err: :close end let(:submodule_git_file) { File.join(submodule, '.git') } From 9c3d118b8f21ef8bf77d9a9dfc62a8051f8078d3 Mon Sep 17 00:00:00 2001 From: Sorin Guga Date: Sun, 25 Feb 2024 01:58:04 +0200 Subject: [PATCH 3/3] Green CI PoC (#833) Cherry-picked commits from #830, #831, #832, #835, #837 and #839. --- .github/workflows/lint.yml | 2 +- .github/workflows/tests.yml | 29 ++++++++----------- Gemfile | 6 +++- .../pre_commit/rails_schema_up_to_date.rb | 2 +- spec/overcommit/git_repo_spec.rb | 20 ++++++++----- .../hook/prepare_commit_msg/base_spec.rb | 6 ++-- .../hook_context/commit_msg_spec.rb | 10 +++---- .../hook_context/pre_commit_spec.rb | 10 +++---- spec/overcommit/installer_spec.rb | 4 +-- 9 files changed, 46 insertions(+), 43 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 8b23cbab..8cf8b4a0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Ruby uses: ruby/setup-ruby@v1 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 26c20dd1..e9987e06 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,25 +14,20 @@ jobs: fail-fast: false matrix: ruby-version: - - '2.6' - - '2.7' - - '3.0' - - '3.1' - - '3.2' + - "2.6" + - "2.7" + - "3.0" + - "3.1" + - "3.2" os: - ubuntu - - windows - - # Tempfile behavior has changed on Ruby 3.1 such that tests - # fail with permission denied. Would welcome a PR with a fix. - exclude: - - ruby-version: '3.1' - os: windows - - ruby-version: '3.2' - os: windows + # At the moment of this commit various specs fail on Windows. + # Any contributor is welcome to fix them and enable the Windows build. + # Please see Issue #836 for more details. + # - windows steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Ruby ${{ matrix.ruby-version }} uses: ruby/setup-ruby@v1 @@ -47,7 +42,7 @@ jobs: bundle exec rspec - name: Code coverage reporting - uses: coverallsapp/github-action@master + uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.github_token }} flag-name: ruby${{ matrix.ruby-version }}-${{ matrix.os }} @@ -59,7 +54,7 @@ jobs: steps: - name: Finalize code coverage report - uses: coverallsapp/github-action@master + uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.github_token }} parallel-finished: true diff --git a/Gemfile b/Gemfile index 53b13c7f..49dc7923 100644 --- a/Gemfile +++ b/Gemfile @@ -12,6 +12,10 @@ gem 'simplecov', '~> 0.21.0' gem 'simplecov-lcov', '~> 0.8.0' # Pin RuboCop for CI builds -gem 'rubocop', '1.59.0' +if RUBY_VERSION < '2.7.0' + gem 'rubocop', '1.50.0' +else + gem 'rubocop', '1.59.0' +end gem 'ffi' if Gem.win_platform? diff --git a/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb b/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb index dafd31a1..61725074 100644 --- a/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb +++ b/lib/overcommit/hook/pre_commit/rails_schema_up_to_date.rb @@ -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 diff --git a/spec/overcommit/git_repo_spec.rb b/spec/overcommit/git_repo_spec.rb index 128737c3..b887415b 100644 --- a/spec/overcommit/git_repo_spec.rb +++ b/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 -c protocol.file.allow=always 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 -c protocol.file.allow=always 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 -c protocol.file.allow=always submodule add \ + #{another_submodule} another-sub-repo 2>&1 > #{File::NULL}` end it { should be_empty } @@ -369,7 +372,8 @@ `git commit --allow-empty -m "Another submodule"` end - `git submodule add #{another_submodule} yet-another-sub-repo 2>&1 > #{File::NULL}` + `git -c protocol.file.allow=always submodule add \ + #{another_submodule} 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` diff --git a/spec/overcommit/hook/prepare_commit_msg/base_spec.rb b/spec/overcommit/hook/prepare_commit_msg/base_spec.rb index 616d6b8e..873ed637 100644 --- a/spec/overcommit/hook/prepare_commit_msg/base_spec.rb +++ b/spec/overcommit/hook/prepare_commit_msg/base_spec.rb @@ -38,9 +38,9 @@ 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 } + t1 = Thread.new { hook_1.run } + t2 = Thread.new { hook_2.run } + [t1, t2].each(&:join) expect(File.read(tempfile)).to match(/alpha\n#{initial_content}bravo\n/m) end end diff --git a/spec/overcommit/hook_context/commit_msg_spec.rb b/spec/overcommit/hook_context/commit_msg_spec.rb index 2aa68516..058f1508 100644 --- a/spec/overcommit/hook_context/commit_msg_spec.rb +++ b/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 diff --git a/spec/overcommit/hook_context/pre_commit_spec.rb b/spec/overcommit/hook_context/pre_commit_spec.rb index dae42e66..719982a1 100644 --- a/spec/overcommit/hook_context/pre_commit_spec.rb +++ b/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 diff --git a/spec/overcommit/installer_spec.rb b/spec/overcommit/installer_spec.rb index c7cfb05e..5283b2b6 100644 --- a/spec/overcommit/installer_spec.rb +++ b/spec/overcommit/installer_spec.rb @@ -231,8 +231,8 @@ 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', - chdir: target, out: :close, err: :close + 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') } let(:submodule_git_dir) do