Skip to content

Commit

Permalink
Fix AuthorName/AuthorEmail to respect env variables
Browse files Browse the repository at this point in the history
When committing, the `GIT_AUTHOR_{NAME,EMAIL}` environment variables
take precedence over Git configuration values. Ensure these are
recognized by the hook.

This required a change to the commit integration spec since
GIT_AUTHOR_NAME/GIT_AUTHOR_EMAIL is set by git before it executes the
hook, so even if you set the local configuration to an empty string it
will fall back to using the global configuration.
  • Loading branch information
sds committed Mar 28, 2016
1 parent af8257f commit 42bb495
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 52 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Overcommit Changelog

## master (unreleased)

* Fix `AuthorName`/`AuthorEmail` pre-commit hooks to respect
`GIT_AUTHOR_NAME`/`GIT_AUTHOR_EMAIL` environment variables, respectively

## 0.32.0

### New Features
Expand Down
12 changes: 9 additions & 3 deletions lib/overcommit/hook/pre_commit/author_email.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@ module Overcommit::Hook::PreCommit
# Checks the format of an author's email address.
class AuthorEmail < Base
def run
result = execute(%w[git config --get user.email])
email = result.stdout.chomp
email =
if ENV.key?('GIT_AUTHOR_EMAIL')
ENV['GIT_AUTHOR_EMAIL']
else
result = execute(%w[git config --get user.email])
result.stdout.chomp
end

unless email =~ /#{config['pattern']}/
return :fail,
"Author has an invalid email address: '#{email}'\n" \
'Set your email with ' \
'`git config --global user.email [email protected]`'
'`git config --global user.email [email protected]` ' \
'or via the GIT_AUTHOR_EMAIL environment variable'
end

:pass
Expand Down
12 changes: 9 additions & 3 deletions lib/overcommit/hook/pre_commit/author_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,19 @@ module Overcommit::Hook::PreCommit
# Ensures that a commit author has a name with at least first and last names.
class AuthorName < Base
def run
result = execute(%w[git config --get user.name])
name = result.stdout.chomp
name =
if ENV.key?('GIT_AUTHOR_NAME')
ENV['GIT_AUTHOR_NAME']
else
result = execute(%w[git config --get user.name])
result.stdout.chomp
end

unless name.split(' ').count >= 2
return :fail,
"Author must have at least first and last name, but was: #{name}.\n" \
'Set your name with `git config --global user.name "Your Name"`'
'Set your name with `git config --global user.name "Your Name"` ' \
'or via the GIT_AUTHOR_NAME environment variable'
end

:pass
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/committing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

context 'when a hook fails' do
before do
`git config --local user.name ""`
`git config --local user.name "John"`
end

it 'exits with a non-zero status' do
Expand Down
80 changes: 48 additions & 32 deletions spec/overcommit/hook/pre_commit/author_email_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,50 +6,66 @@
subject { described_class.new(config, context) }
let(:result) { double('result') }

before do
result.stub(:stdout).and_return(email)
subject.stub(:execute).and_return(result)
end

context 'when user has no email' do
let(:email) { '' }
shared_examples_for 'author email check' do
context 'when user has no email' do
let(:email) { '' }

it { should fail_hook }
end
it { should fail_hook }
end

context 'when user has an invalid email' do
let(:email) { 'Invalid Email' }
context 'when user has an invalid email' do
let(:email) { 'Invalid Email' }

it { should fail_hook }
end
it { should fail_hook }
end

context 'when user has a valid email' do
let(:email) { '[email protected]' }
context 'when user has a valid email' do
let(:email) { '[email protected]' }

it { should pass }
end
it { should pass }
end

context 'when a custom pattern is specified' do
let(:config) do
super().merge(Overcommit::Configuration.new(
'PreCommit' => {
'AuthorEmail' => {
'pattern' => '^[^@]+@brigade\.com$'
context 'when a custom pattern is specified' do
let(:config) do
super().merge(Overcommit::Configuration.new(
'PreCommit' => {
'AuthorEmail' => {
'pattern' => '^[^@]+@brigade\.com$'
}
}
}
))
end
))
end

context 'and the email does not match the pattern' do
let(:email) { '[email protected]' }
context 'and the email does not match the pattern' do
let(:email) { '[email protected]' }

it { should fail_hook }
it { should fail_hook }
end

context 'and the email matches the pattern' do
let(:email) { '[email protected]' }

it { should pass }
end
end
end

context 'and the email matches the pattern' do
let(:email) { '[email protected]' }
context 'when email is set via config' do
before do
result.stub(:stdout).and_return(email)
subject.stub(:execute).and_return(result)
end

it { should pass }
it_should_behave_like 'author email check'
end

context 'when email is set via environment variable' do
around do |example|
Overcommit::Utils.with_environment 'GIT_AUTHOR_EMAIL' => email do
example.run
end
end

it_should_behave_like 'author email check'
end
end
42 changes: 29 additions & 13 deletions spec/overcommit/hook/pre_commit/author_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,42 @@
subject { described_class.new(config, context) }
let(:result) { double('result') }

before do
result.stub(:stdout).and_return(name)
subject.stub(:execute).and_return(result)
end
shared_examples_for 'author name check' do
context 'when user has no name' do
let(:name) { '' }

it { should fail_hook }
end

context 'when user has only a first name' do
let(:name) { 'John' }

it { should fail_hook }
end

context 'when user has no name' do
let(:name) { '' }
context 'when user has first and last name' do
let(:name) { 'John Doe' }

it { should fail_hook }
it { should pass }
end
end

context 'when user has only a first name' do
let(:name) { 'John' }
context 'when name is set via config' do
before do
result.stub(:stdout).and_return(name)
subject.stub(:execute).and_return(result)
end

it { should fail_hook }
it_should_behave_like 'author name check'
end

context 'when user has first and last name' do
let(:name) { 'John Doe' }
context 'when name is set via environment variable' do
around do |example|
Overcommit::Utils.with_environment 'GIT_AUTHOR_NAME' => name do
example.run
end
end

it { should pass }
it_should_behave_like 'author name check'
end
end

0 comments on commit 42bb495

Please sign in to comment.