Skip to content

Commit 3db733e

Browse files
authored
Restore ability to specify "gemfile: false" in config (#863)
Setting `gemfile: false` in `.overcommit.yml` is supposed to disable Bundler. However, a recently-introduced bug causes `false` to be interpreted as the name of the gemfile. Bundler looks for a gemfile named "false", which fails, leading overcommit's hooks to crash. This PR fixes the bug by adjusting the regex used to parse the `gemfile:` line in the config. Now, `false` is no longer interpreted as a gemfile name. I added an integration test to verify the fix. Fixes #862
1 parent 43e17fb commit 3db733e

11 files changed

+144
-87
lines changed

spec/integration/gemfile_option_spec.rb

+134-77
Original file line numberDiff line numberDiff line change
@@ -3,99 +3,156 @@
33
require 'spec_helper'
44

55
describe 'specifying `gemfile` option in Overcommit configuration' do
6-
let(:repo_root) { File.expand_path(File.join('..', '..'), File.dirname(__FILE__)) }
7-
let(:fake_gem_path) { File.join('lib', 'my_fake_gem') }
8-
9-
# We point the overcommit gem back to this repo since we can't assume the gem
10-
# has already been installed in a test environment
11-
let(:gemfile) { normalize_indent(<<-RUBY) }
12-
source 'https://rubygems.org'
13-
14-
gem 'overcommit', path: '#{repo_root}'
15-
gem 'my_fake_gem', path: '#{fake_gem_path}'
16-
gem 'ffi' if Gem.win_platform? # Necessary for test to pass on Windows
17-
RUBY
18-
19-
let(:gemspec) { normalize_indent(<<-RUBY) }
20-
Gem::Specification.new do |s|
21-
s.name = 'my_fake_gem'
22-
s.version = '1.0.0'
23-
s.author = 'John Doe'
24-
s.license = 'MIT'
25-
s.homepage = 'https://example.com'
26-
s.email = 'john.doe@example.com'
27-
s.summary = 'A fake gem'
28-
s.files = [File.join('lib', 'my_fake_gem.rb')]
29-
end
30-
RUBY
31-
32-
# Specify a hook that depends on an external gem to test Gemfile loading
33-
let(:hook) { normalize_indent(<<-RUBY) }
34-
module Overcommit::Hook::PreCommit
35-
class FakeHook < Base
36-
def run
37-
require 'my_fake_gem'
38-
:pass
6+
context 'given a project that uses a Gemfile' do
7+
let(:repo_root) { File.expand_path(File.join('..', '..'), File.dirname(__FILE__)) }
8+
let(:fake_gem_path) { File.join('lib', 'my_fake_gem') }
9+
10+
# We point the overcommit gem back to this repo since we can't assume the gem
11+
# has already been installed in a test environment
12+
let(:gemfile) { normalize_indent(<<-RUBY) }
13+
source 'https://rubygems.org'
14+
15+
gem 'overcommit', path: '#{repo_root}'
16+
gem 'my_fake_gem', path: '#{fake_gem_path}'
17+
gem 'ffi' if Gem.win_platform? # Necessary for test to pass on Windows
18+
RUBY
19+
20+
let(:gemspec) { normalize_indent(<<-RUBY) }
21+
Gem::Specification.new do |s|
22+
s.name = 'my_fake_gem'
23+
s.version = '1.0.0'
24+
s.author = 'John Doe'
25+
s.license = 'MIT'
26+
s.homepage = 'https://example.com'
27+
s.email = 'john.doe@example.com'
28+
s.summary = 'A fake gem'
29+
s.files = [File.join('lib', 'my_fake_gem.rb')]
30+
end
31+
RUBY
32+
33+
# Specify a hook that depends on an external gem to test Gemfile loading
34+
let(:hook) { normalize_indent(<<-RUBY) }
35+
module Overcommit::Hook::PreCommit
36+
class FakeHook < Base
37+
def run
38+
require 'my_fake_gem'
39+
:pass
40+
end
41+
end
42+
end
43+
RUBY
44+
45+
let(:config) { normalize_indent(<<-YAML) }
46+
verify_signatures: false
47+
48+
CommitMsg:
49+
ALL:
50+
enabled: false
51+
52+
PreCommit:
53+
ALL:
54+
enabled: false
55+
FakeHook:
56+
enabled: true
57+
requires_files: false
58+
YAML
59+
60+
around do |example|
61+
repo do
62+
# Since RSpec is being run within a Bundler context we need to clear it
63+
# in order to not taint the test
64+
Bundler.with_unbundled_env do
65+
FileUtils.mkdir_p(File.join(fake_gem_path, 'lib'))
66+
echo(gemspec, File.join(fake_gem_path, 'my_fake_gem.gemspec'))
67+
touch(File.join(fake_gem_path, 'lib', 'my_fake_gem.rb'))
68+
69+
echo(gemfile, '.overcommit_gems.rb')
70+
`bundle install --gemfile=.overcommit_gems.rb`
71+
72+
echo(config, '.overcommit.yml')
73+
74+
# Set BUNDLE_GEMFILE so we load Overcommit from the current repo
75+
ENV['BUNDLE_GEMFILE'] = '.overcommit_gems.rb'
76+
`bundle exec overcommit --install > #{File::NULL}`
77+
FileUtils.mkdir_p(File.join('.git-hooks', 'pre_commit'))
78+
echo(hook, File.join('.git-hooks', 'pre_commit', 'fake_hook.rb'))
79+
80+
Overcommit::Utils.with_environment 'OVERCOMMIT_NO_VERIFY' => '1' do
81+
example.run
82+
end
3983
end
4084
end
4185
end
42-
RUBY
43-
44-
let(:config) { normalize_indent(<<-YAML) }
45-
verify_signatures: false
46-
47-
CommitMsg:
48-
ALL:
49-
enabled: false
50-
51-
PreCommit:
52-
ALL:
53-
enabled: false
54-
FakeHook:
55-
enabled: true
56-
requires_files: false
57-
YAML
58-
59-
around do |example|
60-
repo do
61-
# Since RSpec is being run within a Bundler context we need to clear it
62-
# in order to not taint the test
63-
Bundler.with_unbundled_env do
64-
FileUtils.mkdir_p(File.join(fake_gem_path, 'lib'))
65-
echo(gemspec, File.join(fake_gem_path, 'my_fake_gem.gemspec'))
66-
touch(File.join(fake_gem_path, 'lib', 'my_fake_gem.rb'))
67-
68-
echo(gemfile, '.overcommit_gems.rb')
69-
`bundle install --gemfile=.overcommit_gems.rb`
7086

87+
subject { shell(%w[git commit --allow-empty -m Test]) }
88+
89+
context 'when configuration specifies the gemfile' do
90+
let(:config) { "gemfile: .overcommit_gems.rb\n" + super() }
91+
92+
it 'runs the hook successfully' do
93+
subject.status.should == 0
94+
end
95+
end
96+
97+
context 'when configuration does not specify the gemfile' do
98+
it 'fails to run the hook' do
99+
subject.status.should_not == 0
100+
end
101+
end
102+
end
103+
104+
context 'given a project that does not use a Gemfile' do
105+
let(:hook) { normalize_indent(<<-RUBY) }
106+
module Overcommit::Hook::PreCommit
107+
class NoInvalidGemfileHook < Base
108+
def run
109+
if (gemfile = ENV["BUNDLE_GEMFILE"])
110+
raise unless File.exist?(gemfile)
111+
end
112+
113+
:pass
114+
end
115+
end
116+
end
117+
RUBY
118+
119+
let(:config) { normalize_indent(<<-YAML) }
120+
verify_signatures: false
121+
122+
CommitMsg:
123+
ALL:
124+
enabled: false
125+
126+
PreCommit:
127+
ALL:
128+
enabled: false
129+
NoInvalidGemfileHook:
130+
enabled: true
131+
requires_files: false
132+
YAML
133+
134+
around do |example|
135+
repo do
71136
echo(config, '.overcommit.yml')
72137

73-
# Set BUNDLE_GEMFILE so we load Overcommit from the current repo
74-
ENV['BUNDLE_GEMFILE'] = '.overcommit_gems.rb'
75-
`bundle exec overcommit --install > #{File::NULL}`
138+
`overcommit --install > #{File::NULL}`
76139
FileUtils.mkdir_p(File.join('.git-hooks', 'pre_commit'))
77-
echo(hook, File.join('.git-hooks', 'pre_commit', 'fake_hook.rb'))
140+
echo(hook, File.join('.git-hooks', 'pre_commit', 'no_invalid_gemfile_hook.rb'))
78141

79142
Overcommit::Utils.with_environment 'OVERCOMMIT_NO_VERIFY' => '1' do
80143
example.run
81144
end
82145
end
83146
end
84-
end
85-
86-
subject { shell(%w[git commit --allow-empty -m Test]) }
87147

88-
context 'when configuration specifies the gemfile' do
89-
let(:config) { "gemfile: .overcommit_gems.rb\n" + super() }
148+
subject { shell(%w[git commit --allow-empty -m Test]) }
90149

91-
it 'runs the hook successfully' do
92-
subject.status.should == 0
93-
end
94-
end
150+
context 'when configuration explicitly sets the gemfile to false' do
151+
let(:config) { "gemfile: false\n" + super() }
95152

96-
context 'when configuration does not specify the gemfile' do
97-
it 'fails to run the hook' do
98-
subject.status.should_not == 0
153+
it 'runs the hook successfully' do
154+
subject.status.should == 0
155+
end
99156
end
100157
end
101158
end

template-dir/hooks/commit-msg

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/overcommit-hook

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/post-checkout

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/post-commit

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/post-merge

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/post-rewrite

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/pre-commit

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/pre-push

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/pre-rebase

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

template-dir/hooks/prepare-commit-msg

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if hook_type == 'overcommit-hook'
2727
end
2828

2929
# Check if Overcommit should invoke a Bundler context for loading gems
30-
config = File.read('.overcommit.yml') =~ /gemfile: ['"]?(.*)['"]?/
30+
File.read('.overcommit.yml') =~ /gemfile: (?:false|['"]?(.*)['"]?)/
3131
gemfile = Regexp.last_match(1)
3232

3333
if gemfile

0 commit comments

Comments
 (0)