From 8a6dc9d659bd0e42fef70d5482352226173d7342 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Fri, 14 Feb 2025 23:05:08 -0800 Subject: [PATCH 1/3] Add context and indent --- spec/integration/gemfile_option_spec.rb | 168 ++++++++++++------------ 1 file changed, 85 insertions(+), 83 deletions(-) diff --git a/spec/integration/gemfile_option_spec.rb b/spec/integration/gemfile_option_spec.rb index 7a0175da..7aa0629e 100644 --- a/spec/integration/gemfile_option_spec.rb +++ b/spec/integration/gemfile_option_spec.rb @@ -3,99 +3,101 @@ require 'spec_helper' describe 'specifying `gemfile` option in Overcommit configuration' do - let(:repo_root) { File.expand_path(File.join('..', '..'), File.dirname(__FILE__)) } - let(:fake_gem_path) { File.join('lib', 'my_fake_gem') } - - # We point the overcommit gem back to this repo since we can't assume the gem - # has already been installed in a test environment - let(:gemfile) { normalize_indent(<<-RUBY) } - source 'https://rubygems.org' - - gem 'overcommit', path: '#{repo_root}' - gem 'my_fake_gem', path: '#{fake_gem_path}' - gem 'ffi' if Gem.win_platform? # Necessary for test to pass on Windows - RUBY - - let(:gemspec) { normalize_indent(<<-RUBY) } - Gem::Specification.new do |s| - s.name = 'my_fake_gem' - s.version = '1.0.0' - s.author = 'John Doe' - s.license = 'MIT' - s.homepage = 'https://example.com' - s.email = 'john.doe@example.com' - s.summary = 'A fake gem' - s.files = [File.join('lib', 'my_fake_gem.rb')] - end - RUBY - - # Specify a hook that depends on an external gem to test Gemfile loading - let(:hook) { normalize_indent(<<-RUBY) } - module Overcommit::Hook::PreCommit - class FakeHook < Base - def run - require 'my_fake_gem' - :pass + context 'given a project that uses a Gemfile' do + let(:repo_root) { File.expand_path(File.join('..', '..'), File.dirname(__FILE__)) } + let(:fake_gem_path) { File.join('lib', 'my_fake_gem') } + + # We point the overcommit gem back to this repo since we can't assume the gem + # has already been installed in a test environment + let(:gemfile) { normalize_indent(<<-RUBY) } + source 'https://rubygems.org' + + gem 'overcommit', path: '#{repo_root}' + gem 'my_fake_gem', path: '#{fake_gem_path}' + gem 'ffi' if Gem.win_platform? # Necessary for test to pass on Windows + RUBY + + let(:gemspec) { normalize_indent(<<-RUBY) } + Gem::Specification.new do |s| + s.name = 'my_fake_gem' + s.version = '1.0.0' + s.author = 'John Doe' + s.license = 'MIT' + s.homepage = 'https://example.com' + s.email = 'john.doe@example.com' + s.summary = 'A fake gem' + s.files = [File.join('lib', 'my_fake_gem.rb')] + end + RUBY + + # Specify a hook that depends on an external gem to test Gemfile loading + let(:hook) { normalize_indent(<<-RUBY) } + module Overcommit::Hook::PreCommit + class FakeHook < Base + def run + require 'my_fake_gem' + :pass + end end end - end - RUBY - - let(:config) { normalize_indent(<<-YAML) } - verify_signatures: false - - CommitMsg: - ALL: - enabled: false - - PreCommit: - ALL: - enabled: false - FakeHook: - enabled: true - requires_files: false - YAML - - around do |example| - repo do - # Since RSpec is being run within a Bundler context we need to clear it - # in order to not taint the test - Bundler.with_unbundled_env do - FileUtils.mkdir_p(File.join(fake_gem_path, 'lib')) - echo(gemspec, File.join(fake_gem_path, 'my_fake_gem.gemspec')) - touch(File.join(fake_gem_path, 'lib', 'my_fake_gem.rb')) - - echo(gemfile, '.overcommit_gems.rb') - `bundle install --gemfile=.overcommit_gems.rb` - - echo(config, '.overcommit.yml') - - # Set BUNDLE_GEMFILE so we load Overcommit from the current repo - ENV['BUNDLE_GEMFILE'] = '.overcommit_gems.rb' - `bundle exec overcommit --install > #{File::NULL}` - FileUtils.mkdir_p(File.join('.git-hooks', 'pre_commit')) - echo(hook, File.join('.git-hooks', 'pre_commit', 'fake_hook.rb')) - - Overcommit::Utils.with_environment 'OVERCOMMIT_NO_VERIFY' => '1' do - example.run + RUBY + + let(:config) { normalize_indent(<<-YAML) } + verify_signatures: false + + CommitMsg: + ALL: + enabled: false + + PreCommit: + ALL: + enabled: false + FakeHook: + enabled: true + requires_files: false + YAML + + around do |example| + repo do + # Since RSpec is being run within a Bundler context we need to clear it + # in order to not taint the test + Bundler.with_unbundled_env do + FileUtils.mkdir_p(File.join(fake_gem_path, 'lib')) + echo(gemspec, File.join(fake_gem_path, 'my_fake_gem.gemspec')) + touch(File.join(fake_gem_path, 'lib', 'my_fake_gem.rb')) + + echo(gemfile, '.overcommit_gems.rb') + `bundle install --gemfile=.overcommit_gems.rb` + + echo(config, '.overcommit.yml') + + # Set BUNDLE_GEMFILE so we load Overcommit from the current repo + ENV['BUNDLE_GEMFILE'] = '.overcommit_gems.rb' + `bundle exec overcommit --install > #{File::NULL}` + FileUtils.mkdir_p(File.join('.git-hooks', 'pre_commit')) + echo(hook, File.join('.git-hooks', 'pre_commit', 'fake_hook.rb')) + + Overcommit::Utils.with_environment 'OVERCOMMIT_NO_VERIFY' => '1' do + example.run + end end end end - end - subject { shell(%w[git commit --allow-empty -m Test]) } + subject { shell(%w[git commit --allow-empty -m Test]) } - context 'when configuration specifies the gemfile' do - let(:config) { "gemfile: .overcommit_gems.rb\n" + super() } + context 'when configuration specifies the gemfile' do + let(:config) { "gemfile: .overcommit_gems.rb\n" + super() } - it 'runs the hook successfully' do - subject.status.should == 0 + it 'runs the hook successfully' do + subject.status.should == 0 + end end - end - context 'when configuration does not specify the gemfile' do - it 'fails to run the hook' do - subject.status.should_not == 0 + context 'when configuration does not specify the gemfile' do + it 'fails to run the hook' do + subject.status.should_not == 0 + end end end end From 3c11e55f5e77f7e5a1ad34094f2fad07d49ec287 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Fri, 14 Feb 2025 23:59:21 -0800 Subject: [PATCH 2/3] Add failing spec --- spec/integration/gemfile_option_spec.rb | 55 +++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/spec/integration/gemfile_option_spec.rb b/spec/integration/gemfile_option_spec.rb index 7aa0629e..a6a6a4ba 100644 --- a/spec/integration/gemfile_option_spec.rb +++ b/spec/integration/gemfile_option_spec.rb @@ -100,4 +100,59 @@ def run end end end + + context 'given a project that does not use a Gemfile' do + let(:hook) { normalize_indent(<<-RUBY) } + module Overcommit::Hook::PreCommit + class NoInvalidGemfileHook < Base + def run + if (gemfile = ENV["BUNDLE_GEMFILE"]) + raise unless File.exist?(gemfile) + end + + :pass + end + end + end + RUBY + + let(:config) { normalize_indent(<<-YAML) } + verify_signatures: false + + CommitMsg: + ALL: + enabled: false + + PreCommit: + ALL: + enabled: false + NoInvalidGemfileHook: + enabled: true + requires_files: false + YAML + + around do |example| + repo do + echo(config, '.overcommit.yml') + + `overcommit --install > #{File::NULL}` + FileUtils.mkdir_p(File.join('.git-hooks', 'pre_commit')) + echo(hook, File.join('.git-hooks', 'pre_commit', 'no_invalid_gemfile_hook.rb')) + + Overcommit::Utils.with_environment 'OVERCOMMIT_NO_VERIFY' => '1' do + example.run + end + end + end + + subject { shell(%w[git commit --allow-empty -m Test]) } + + context 'when configuration explicitly sets the gemfile to false' do + let(:config) { "gemfile: false\n" + super() } + + it 'runs the hook successfully' do + subject.status.should == 0 + end + end + end end From a4d42e645cd577f4f5b74d3b5d243cf33164a3b1 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Fri, 14 Feb 2025 23:59:30 -0800 Subject: [PATCH 3/3] Update regex to properly handle YAML false --- template-dir/hooks/commit-msg | 2 +- template-dir/hooks/overcommit-hook | 2 +- template-dir/hooks/post-checkout | 2 +- template-dir/hooks/post-commit | 2 +- template-dir/hooks/post-merge | 2 +- template-dir/hooks/post-rewrite | 2 +- template-dir/hooks/pre-commit | 2 +- template-dir/hooks/pre-push | 2 +- template-dir/hooks/pre-rebase | 2 +- template-dir/hooks/prepare-commit-msg | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/template-dir/hooks/commit-msg b/template-dir/hooks/commit-msg index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/commit-msg +++ b/template-dir/hooks/commit-msg @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/overcommit-hook b/template-dir/hooks/overcommit-hook index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/overcommit-hook +++ b/template-dir/hooks/overcommit-hook @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/post-checkout b/template-dir/hooks/post-checkout index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/post-checkout +++ b/template-dir/hooks/post-checkout @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/post-commit b/template-dir/hooks/post-commit index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/post-commit +++ b/template-dir/hooks/post-commit @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/post-merge b/template-dir/hooks/post-merge index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/post-merge +++ b/template-dir/hooks/post-merge @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/post-rewrite b/template-dir/hooks/post-rewrite index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/post-rewrite +++ b/template-dir/hooks/post-rewrite @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/pre-commit b/template-dir/hooks/pre-commit index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/pre-commit +++ b/template-dir/hooks/pre-commit @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/pre-push b/template-dir/hooks/pre-push index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/pre-push +++ b/template-dir/hooks/pre-push @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/pre-rebase b/template-dir/hooks/pre-rebase index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/pre-rebase +++ b/template-dir/hooks/pre-rebase @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile diff --git a/template-dir/hooks/prepare-commit-msg b/template-dir/hooks/prepare-commit-msg index 377c892b..87ffeb58 100755 --- a/template-dir/hooks/prepare-commit-msg +++ b/template-dir/hooks/prepare-commit-msg @@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook' end # Check if Overcommit should invoke a Bundler context for loading gems -config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/ +File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/ gemfile = Regexp.last_match(1) if gemfile