diff --git a/lib/overcommit/hook_context/base.rb b/lib/overcommit/hook_context/base.rb index 4e3c8727..a68f1245 100644 --- a/lib/overcommit/hook_context/base.rb +++ b/lib/overcommit/hook_context/base.rb @@ -94,5 +94,30 @@ def input_string def input_lines @input_lines ||= input_string.split("\n") end + + private + + def filter_modified_files(modified_files) + filter_directories(filter_nonexistent(modified_files)) + end + + # Filter out non-existent files (unless it's a broken symlink, in which case + # it's a file that points to a non-existent file). This could happen if a + # file was renamed as part of an amendment, leading to the old file no + # longer existing. + def filter_nonexistent(modified_files) + modified_files.select do |file| + File.exist?(file) || Overcommit::Utils.broken_symlink?(file) + end + end + + # Filter out directories. This could happen when changing a symlink to a + # directory as part of an amendment, since the symlink will still appear as + # a file, but the actual working tree will have a directory. + def filter_directories(modified_files) + modified_files.reject do |file| + File.directory?(file) && !File.symlink?(file) + end + end end end diff --git a/lib/overcommit/hook_context/post_rewrite.rb b/lib/overcommit/hook_context/post_rewrite.rb index 7e87c6c7..249071dc 100644 --- a/lib/overcommit/hook_context/post_rewrite.rb +++ b/lib/overcommit/hook_context/post_rewrite.rb @@ -25,6 +25,22 @@ def rewritten_commits end end + # Get a list of files that have been added or modified as part of a + # rewritten commit. Renames and deletions are ignored, since there should be + # nothing to check. + def modified_files + @modified_files ||= begin + @modified_files = [] + + rewritten_commits.each do |rewritten_commit| + refs = "#{rewritten_commit.old_hash} #{rewritten_commit.new_hash}" + @modified_files |= Overcommit::GitRepo.modified_files(refs: refs) + end + + filter_modified_files(@modified_files) + end + end + # Struct encapsulating the old and new SHA1 hashes of a rewritten commit RewrittenCommit = Struct.new(:old_hash, :new_hash) end diff --git a/lib/overcommit/hook_context/pre_commit.rb b/lib/overcommit/hook_context/pre_commit.rb index a315b2e3..212c539c 100644 --- a/lib/overcommit/hook_context/pre_commit.rb +++ b/lib/overcommit/hook_context/pre_commit.rb @@ -95,23 +95,7 @@ def modified_files if amendment? subcmd = 'show --format=%n' previously_modified = Overcommit::GitRepo.modified_files(subcmd: subcmd) - - # Filter out non-existent files (unless it's a broken symlink, in - # which case it's a file that points to a non-existent file). - # This could happen if a file was renamed as part of the amendment, - # leading to the old file no longer existing. - previously_modified.select! do |file| - File.exist?(file) || Overcommit::Utils.broken_symlink?(file) - end - - # Filter out directories. This could happen when changing a symlink to - # a directory as part of an amendment, since the symlink will still - # appear as a file, but the actual working tree will have a directory. - previously_modified.reject! do |file| - File.directory?(file) && !File.symlink?(file) - end - - @modified_files |= previously_modified + @modified_files |= filter_modified_files(previously_modified) end end @modified_files diff --git a/spec/overcommit/hook_context/post_rewrite_spec.rb b/spec/overcommit/hook_context/post_rewrite_spec.rb index 231d3bd1..7bb3583b 100644 --- a/spec/overcommit/hook_context/post_rewrite_spec.rb +++ b/spec/overcommit/hook_context/post_rewrite_spec.rb @@ -79,4 +79,144 @@ end end end + + describe '#modified_files' do + subject { context.modified_files } + + before do + context.stub(:rewritten_commits).and_return(rewritten_commits) + end + + context 'when rewrite was triggered by amend' do + let(:args) { ['amend'] } + let(:rewritten_commits) { [double(old_hash: 'HEAD@{1}', new_hash: 'HEAD')] } + + it 'does not include submodules' do + submodule = repo do + FileUtils.touch 'foo' + `git add foo` + `git commit -m "Initial commit"` + end + + repo do + `git commit --allow-empty -m "Initial commit"` + `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 + end + + context 'when no files were modified' do + around do |example| + repo do + `git commit --allow-empty -m "Initial commit"` + `git commit --amend --allow-empty -m "Another commit"` + example.run + end + end + + it { should be_empty } + end + + context 'when files were added' do + around do |example| + repo do + `git commit --allow-empty -m "Initial commit"` + FileUtils.touch('some-file') + `git add some-file` + `git commit --amend -m "Add file"` + example.run + end + end + + it { should == [File.expand_path('some-file')] } + end + + context 'when files were modified' do + around do |example| + repo do + FileUtils.touch('some-file') + `git add some-file` + `git commit -m "Initial commit"` + echo('Hello', 'some-file') + `git add some-file` + `git commit --amend -m "Modify file"` + example.run + end + end + + it { should == [File.expand_path('some-file')] } + end + + context 'when files were deleted' do + around do |example| + repo do + FileUtils.touch('some-file') + `git add some-file` + `git commit -m "Initial commit"` + `git rm some-file` + `git commit --amend --allow-empty -m "Delete file"` + example.run + end + end + + it { should be_empty } + end + + context 'when files were renamed' do + around do |example| + repo do + FileUtils.touch 'some-file' + `git add some-file` + `git commit -m "Add file"` + `git mv some-file renamed-file` + `git commit --amend -m "Rename file"` + example.run + end + end + + it { should == [File.expand_path('renamed-file')] } + end + + context 'when changing a symlink to a directory during an amendment' do + around do |example| + repo do + FileUtils.mkdir 'some-directory' + FileUtils.ln_s 'some-directory', 'some-symlink' + `git add some-symlink some-directory` + `git commit -m "Add file"` + `git rm some-symlink` + FileUtils.mkdir 'some-symlink' + FileUtils.touch File.join('some-symlink', 'another-file') + `git add some-symlink` + `git commit --amend -m "Change symlink to directory"` + example.run + end + end + + it 'does not include the directory in the list of modified files' do + subject.should_not include File.expand_path('some-symlink') + end + end + + context 'when breaking a symlink during an amendment' do + around do |example| + repo do + FileUtils.mkdir 'some-directory' + FileUtils.touch File.join('some-directory', 'some-file') + FileUtils.ln_s 'some-directory', 'some-symlink' + `git add some-symlink some-directory` + `git commit -m "Add file"` + `git rm -rf some-directory` + `git commit --amend -m "Remove directory to break symlink"` + example.run + end + end + + it 'does not include the broken symlink in the list of modified files' do + subject.should_not include File.expand_path('some-symlink') + end + end + end + end end